You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@activemq.apache.org by "Torsten Mielke (JIRA)" <ji...@apache.org> on 2011/09/20 16:04:09 UTC
[jira] [Created] (AMQ-3506) Access to ConnectionPool.createSession
needs to be synchronized
Access to ConnectionPool.createSession needs to be synchronized
----------------------------------------------------------------
Key: AMQ-3506
URL: https://issues.apache.org/jira/browse/AMQ-3506
Project: ActiveMQ
Issue Type: Bug
Components: Broker
Affects Versions: 5.5.0
Environment: activemq-pool, PooledConnectionFactory with maximumActive=1 and blockIfSessionPoolIsFull=true (default behavior)
Reporter: Torsten Mielke
Fix For: 5.6.0
When configuring a PooledConnectionFactory with maximumActive=1 and blockIfSessionPoolIsFull=true (default behavior for latter config) it is possible that multiple threads that concurrently try to use the same JMS connection to create a new session might create more sessions than the configured maximumActive limit.
That's because the call to ConnectionPool.createSession() is not synchronized and if multiple threads try to call this method concurrently (on the same underlying JMS connection) then the if-condition in
{code:java}
SessionKey key = new SessionKey(transacted, ackMode);
SessionPool pool = cache.get(key);
if (pool == null) {
pool = createSessionPool(key);
cache.put(key, pool);
}
{code}
will evaluate to true for *all* threads and they all end up creating their own sessionPool using the same SessionKey properties.
Access to the if-condition needs to be synchronized so that only one session pool gets created. That will ensure that not more than the configured maximumActive number of sessions can get created.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (AMQ-3506) Access to
ConnectionPool.createSession needs to be synchronized
Posted by "Torsten Mielke (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/AMQ-3506?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13109338#comment-13109338 ]
Torsten Mielke commented on AMQ-3506:
-------------------------------------
Claudio's suggestion should also work (using newPool.close() to discard the pool). I tested it against the JUnit tests of activemq-pool and against a Camel route that consumes from JMS.
It has the advantage that once the cache contains a pool object, there is no additional overhead. All overhead is only in the case when the cache does not yet contain the key.
In my solution there is a synchronized call for every createSession() invocation.
> Access to ConnectionPool.createSession needs to be synchronized
> ----------------------------------------------------------------
>
> Key: AMQ-3506
> URL: https://issues.apache.org/jira/browse/AMQ-3506
> Project: ActiveMQ
> Issue Type: Bug
> Components: Broker
> Affects Versions: 5.5.0
> Environment: activemq-pool, PooledConnectionFactory with maximumActive=1 and blockIfSessionPoolIsFull=true (default behavior)
> Reporter: Torsten Mielke
> Labels: activemq-pool, maximumActive, sessionPool
> Fix For: 5.6.0
>
> Attachments: AMQ-3506.patch
>
> Original Estimate: 3h
> Remaining Estimate: 3h
>
> When configuring a PooledConnectionFactory with maximumActive=1 and blockIfSessionPoolIsFull=true (default behavior for latter config) it is possible that multiple threads that concurrently try to use the same JMS connection to create a new session might create more sessions than the configured maximumActive limit.
> That's because the call to ConnectionPool.createSession() is not synchronized and if multiple threads try to call this method concurrently (on the same underlying JMS connection) then the if-condition in
> {code:java}
> SessionKey key = new SessionKey(transacted, ackMode);
> SessionPool pool = cache.get(key);
> if (pool == null) {
> pool = createSessionPool(key);
> cache.put(key, pool);
> }
> {code}
> will evaluate to true for *all* threads and they all end up creating their own sessionPool using the same SessionKey properties.
> Access to the if-condition needs to be synchronized so that only one session pool gets created. That will ensure that not more than the configured maximumActive number of sessions can get created.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (AMQ-3506) Access to
ConnectionPool.createSession needs to be synchronized
Posted by "Torsten Mielke (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/AMQ-3506?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13108727#comment-13108727 ]
Torsten Mielke commented on AMQ-3506:
-------------------------------------
IMHO the fix is to synchronize access to the cache and the creation of the session pool.
{code:java}
public Session createSession(boolean transacted, int ackMode) throws JMSException {
SessionKey key = new SessionKey(transacted, ackMode);
SessionPool pool = null;
synchronized (cache) {
pool = cache.get(key);
if (pool == null) {
pool = createSessionPool(key);
cache.put(key, pool);
}
}
PooledSession session = pool.borrowSession();
return session;
}
{code}
> Access to ConnectionPool.createSession needs to be synchronized
> ----------------------------------------------------------------
>
> Key: AMQ-3506
> URL: https://issues.apache.org/jira/browse/AMQ-3506
> Project: ActiveMQ
> Issue Type: Bug
> Components: Broker
> Affects Versions: 5.5.0
> Environment: activemq-pool, PooledConnectionFactory with maximumActive=1 and blockIfSessionPoolIsFull=true (default behavior)
> Reporter: Torsten Mielke
> Labels: activemq-pool, maximumActive, sessionPool
> Fix For: 5.6.0
>
> Attachments: AMQ-3506.patch
>
> Original Estimate: 3h
> Remaining Estimate: 3h
>
> When configuring a PooledConnectionFactory with maximumActive=1 and blockIfSessionPoolIsFull=true (default behavior for latter config) it is possible that multiple threads that concurrently try to use the same JMS connection to create a new session might create more sessions than the configured maximumActive limit.
> That's because the call to ConnectionPool.createSession() is not synchronized and if multiple threads try to call this method concurrently (on the same underlying JMS connection) then the if-condition in
> {code:java}
> SessionKey key = new SessionKey(transacted, ackMode);
> SessionPool pool = cache.get(key);
> if (pool == null) {
> pool = createSessionPool(key);
> cache.put(key, pool);
> }
> {code}
> will evaluate to true for *all* threads and they all end up creating their own sessionPool using the same SessionKey properties.
> Access to the if-condition needs to be synchronized so that only one session pool gets created. That will ensure that not more than the configured maximumActive number of sessions can get created.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (AMQ-3506) Access to
ConnectionPool.createSession needs to be synchronized
Posted by "Claudio Corsi (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/AMQ-3506?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13108855#comment-13108855 ]
Claudio Corsi commented on AMQ-3506:
------------------------------------
We might consider redefining the cache field from Map to ConcurrentMap and change the code somewhat.
<code:java>
public Session createSession(boolean transacted, int ackMode) throws JMSException {
SessionKey key = new SessionKey(transacted, ackMode);
SessionPool pool = null;
pool = cache.get(key);
if (pool == null) {
SessionPool newPool = createSessionPool(key);
SessionPool prevPool = cache.putIfAbsent(key, newPool);
if (prevPool != null && prevPool != newPool) {
// newPool was not the first one to be associated with this key... close created session pool
newPool.destroy(); // don't remember the method call
}
pool = cache.get(key); // this will return a non-null value...
}
PooledSession session = pool.borrowSession();
return session;
}
<code>
> Access to ConnectionPool.createSession needs to be synchronized
> ----------------------------------------------------------------
>
> Key: AMQ-3506
> URL: https://issues.apache.org/jira/browse/AMQ-3506
> Project: ActiveMQ
> Issue Type: Bug
> Components: Broker
> Affects Versions: 5.5.0
> Environment: activemq-pool, PooledConnectionFactory with maximumActive=1 and blockIfSessionPoolIsFull=true (default behavior)
> Reporter: Torsten Mielke
> Labels: activemq-pool, maximumActive, sessionPool
> Fix For: 5.6.0
>
> Attachments: AMQ-3506.patch
>
> Original Estimate: 3h
> Remaining Estimate: 3h
>
> When configuring a PooledConnectionFactory with maximumActive=1 and blockIfSessionPoolIsFull=true (default behavior for latter config) it is possible that multiple threads that concurrently try to use the same JMS connection to create a new session might create more sessions than the configured maximumActive limit.
> That's because the call to ConnectionPool.createSession() is not synchronized and if multiple threads try to call this method concurrently (on the same underlying JMS connection) then the if-condition in
> {code:java}
> SessionKey key = new SessionKey(transacted, ackMode);
> SessionPool pool = cache.get(key);
> if (pool == null) {
> pool = createSessionPool(key);
> cache.put(key, pool);
> }
> {code}
> will evaluate to true for *all* threads and they all end up creating their own sessionPool using the same SessionKey properties.
> Access to the if-condition needs to be synchronized so that only one session pool gets created. That will ensure that not more than the configured maximumActive number of sessions can get created.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Issue Comment Edited] (AMQ-3506) Access to
ConnectionPool.createSession needs to be synchronized
Posted by "Claudio Corsi (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/AMQ-3506?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13108855#comment-13108855 ]
Claudio Corsi edited comment on AMQ-3506 at 9/20/11 5:48 PM:
-------------------------------------------------------------
We might consider redefining the cache field from Map to ConcurrentMap and change the code somewhat.
<code:java>
public Session createSession(boolean transacted, int ackMode) throws JMSException {
SessionKey key = new SessionKey(transacted, ackMode);
SessionPool pool = null;
pool = cache.get(key);
if (pool == null) {
SessionPool newPool = createSessionPool(key);
SessionPool prevPool = cache.putIfAbsent(key, newPool);
if (prevPool != null && prevPool != newPool) {
// newPool was not the first one to be associated with this key... close created session pool
newPool.destroy(); // don't remember the method call
}
pool = cache.get(key); // this will return a non-null value...
}
PooledSession session = pool.borrowSession();
return session;
}
</code>
was (Author: ccorsi):
We might consider redefining the cache field from Map to ConcurrentMap and change the code somewhat.
<code:java>
public Session createSession(boolean transacted, int ackMode) throws JMSException {
SessionKey key = new SessionKey(transacted, ackMode);
SessionPool pool = null;
pool = cache.get(key);
if (pool == null) {
SessionPool newPool = createSessionPool(key);
SessionPool prevPool = cache.putIfAbsent(key, newPool);
if (prevPool != null && prevPool != newPool) {
// newPool was not the first one to be associated with this key... close created session pool
newPool.destroy(); // don't remember the method call
}
pool = cache.get(key); // this will return a non-null value...
}
PooledSession session = pool.borrowSession();
return session;
}
<code>
> Access to ConnectionPool.createSession needs to be synchronized
> ----------------------------------------------------------------
>
> Key: AMQ-3506
> URL: https://issues.apache.org/jira/browse/AMQ-3506
> Project: ActiveMQ
> Issue Type: Bug
> Components: Broker
> Affects Versions: 5.5.0
> Environment: activemq-pool, PooledConnectionFactory with maximumActive=1 and blockIfSessionPoolIsFull=true (default behavior)
> Reporter: Torsten Mielke
> Labels: activemq-pool, maximumActive, sessionPool
> Fix For: 5.6.0
>
> Attachments: AMQ-3506.patch
>
> Original Estimate: 3h
> Remaining Estimate: 3h
>
> When configuring a PooledConnectionFactory with maximumActive=1 and blockIfSessionPoolIsFull=true (default behavior for latter config) it is possible that multiple threads that concurrently try to use the same JMS connection to create a new session might create more sessions than the configured maximumActive limit.
> That's because the call to ConnectionPool.createSession() is not synchronized and if multiple threads try to call this method concurrently (on the same underlying JMS connection) then the if-condition in
> {code:java}
> SessionKey key = new SessionKey(transacted, ackMode);
> SessionPool pool = cache.get(key);
> if (pool == null) {
> pool = createSessionPool(key);
> cache.put(key, pool);
> }
> {code}
> will evaluate to true for *all* threads and they all end up creating their own sessionPool using the same SessionKey properties.
> Access to the if-condition needs to be synchronized so that only one session pool gets created. That will ensure that not more than the configured maximumActive number of sessions can get created.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (AMQ-3506) Access to ConnectionPool.createSession
needs to be synchronized
Posted by "Torsten Mielke (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/AMQ-3506?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Torsten Mielke updated AMQ-3506:
--------------------------------
Attachment: AMQ-3506.patch
Attaching possible fix and corresponding JUnit test
> Access to ConnectionPool.createSession needs to be synchronized
> ----------------------------------------------------------------
>
> Key: AMQ-3506
> URL: https://issues.apache.org/jira/browse/AMQ-3506
> Project: ActiveMQ
> Issue Type: Bug
> Components: Broker
> Affects Versions: 5.5.0
> Environment: activemq-pool, PooledConnectionFactory with maximumActive=1 and blockIfSessionPoolIsFull=true (default behavior)
> Reporter: Torsten Mielke
> Labels: activemq-pool, maximumActive, sessionPool
> Fix For: 5.6.0
>
> Attachments: AMQ-3506.patch
>
> Original Estimate: 3h
> Remaining Estimate: 3h
>
> When configuring a PooledConnectionFactory with maximumActive=1 and blockIfSessionPoolIsFull=true (default behavior for latter config) it is possible that multiple threads that concurrently try to use the same JMS connection to create a new session might create more sessions than the configured maximumActive limit.
> That's because the call to ConnectionPool.createSession() is not synchronized and if multiple threads try to call this method concurrently (on the same underlying JMS connection) then the if-condition in
> {code:java}
> SessionKey key = new SessionKey(transacted, ackMode);
> SessionPool pool = cache.get(key);
> if (pool == null) {
> pool = createSessionPool(key);
> cache.put(key, pool);
> }
> {code}
> will evaluate to true for *all* threads and they all end up creating their own sessionPool using the same SessionKey properties.
> Access to the if-condition needs to be synchronized so that only one session pool gets created. That will ensure that not more than the configured maximumActive number of sessions can get created.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Issue Comment Edited] (AMQ-3506) Access to
ConnectionPool.createSession needs to be synchronized
Posted by "Claudio Corsi (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/AMQ-3506?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13108855#comment-13108855 ]
Claudio Corsi edited comment on AMQ-3506 at 9/20/11 5:49 PM:
-------------------------------------------------------------
We might consider redefining the cache field from Map to ConcurrentMap and change the code somewhat.
{code:java}
public Session createSession(boolean transacted, int ackMode) throws JMSException {
SessionKey key = new SessionKey(transacted, ackMode);
SessionPool pool = null;
pool = cache.get(key);
if (pool == null) {
SessionPool newPool = createSessionPool(key);
SessionPool prevPool = cache.putIfAbsent(key, newPool);
if (prevPool != null && prevPool != newPool) {
// newPool was not the first one to be associated with this key... close created session pool
newPool.destroy(); // don't remember the method call
}
pool = cache.get(key); // this will return a non-null value...
}
PooledSession session = pool.borrowSession();
return session;
}
{code}
was (Author: ccorsi):
We might consider redefining the cache field from Map to ConcurrentMap and change the code somewhat.
<code:java>
public Session createSession(boolean transacted, int ackMode) throws JMSException {
SessionKey key = new SessionKey(transacted, ackMode);
SessionPool pool = null;
pool = cache.get(key);
if (pool == null) {
SessionPool newPool = createSessionPool(key);
SessionPool prevPool = cache.putIfAbsent(key, newPool);
if (prevPool != null && prevPool != newPool) {
// newPool was not the first one to be associated with this key... close created session pool
newPool.destroy(); // don't remember the method call
}
pool = cache.get(key); // this will return a non-null value...
}
PooledSession session = pool.borrowSession();
return session;
}
</code>
> Access to ConnectionPool.createSession needs to be synchronized
> ----------------------------------------------------------------
>
> Key: AMQ-3506
> URL: https://issues.apache.org/jira/browse/AMQ-3506
> Project: ActiveMQ
> Issue Type: Bug
> Components: Broker
> Affects Versions: 5.5.0
> Environment: activemq-pool, PooledConnectionFactory with maximumActive=1 and blockIfSessionPoolIsFull=true (default behavior)
> Reporter: Torsten Mielke
> Labels: activemq-pool, maximumActive, sessionPool
> Fix For: 5.6.0
>
> Attachments: AMQ-3506.patch
>
> Original Estimate: 3h
> Remaining Estimate: 3h
>
> When configuring a PooledConnectionFactory with maximumActive=1 and blockIfSessionPoolIsFull=true (default behavior for latter config) it is possible that multiple threads that concurrently try to use the same JMS connection to create a new session might create more sessions than the configured maximumActive limit.
> That's because the call to ConnectionPool.createSession() is not synchronized and if multiple threads try to call this method concurrently (on the same underlying JMS connection) then the if-condition in
> {code:java}
> SessionKey key = new SessionKey(transacted, ackMode);
> SessionPool pool = cache.get(key);
> if (pool == null) {
> pool = createSessionPool(key);
> cache.put(key, pool);
> }
> {code}
> will evaluate to true for *all* threads and they all end up creating their own sessionPool using the same SessionKey properties.
> Access to the if-condition needs to be synchronized so that only one session pool gets created. That will ensure that not more than the configured maximumActive number of sessions can get created.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Resolved] (AMQ-3506) Access to ConnectionPool.createSession
needs to be synchronized
Posted by "Timothy Bish (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/AMQ-3506?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Timothy Bish resolved AMQ-3506.
-------------------------------
Resolution: Fixed
Applied supplied test case and used the concurrent map approach over the synchronization option. Please review and reopen if an issue is found with the fix.
> Access to ConnectionPool.createSession needs to be synchronized
> ----------------------------------------------------------------
>
> Key: AMQ-3506
> URL: https://issues.apache.org/jira/browse/AMQ-3506
> Project: ActiveMQ
> Issue Type: Bug
> Components: Broker
> Affects Versions: 5.5.0
> Environment: activemq-pool, PooledConnectionFactory with maximumActive=1 and blockIfSessionPoolIsFull=true (default behavior)
> Reporter: Torsten Mielke
> Labels: activemq-pool, maximumActive, sessionPool
> Fix For: 5.6.0
>
> Attachments: AMQ-3506.patch
>
> Original Estimate: 3h
> Remaining Estimate: 3h
>
> When configuring a PooledConnectionFactory with maximumActive=1 and blockIfSessionPoolIsFull=true (default behavior for latter config) it is possible that multiple threads that concurrently try to use the same JMS connection to create a new session might create more sessions than the configured maximumActive limit.
> That's because the call to ConnectionPool.createSession() is not synchronized and if multiple threads try to call this method concurrently (on the same underlying JMS connection) then the if-condition in
> {code:java}
> SessionKey key = new SessionKey(transacted, ackMode);
> SessionPool pool = cache.get(key);
> if (pool == null) {
> pool = createSessionPool(key);
> cache.put(key, pool);
> }
> {code}
> will evaluate to true for *all* threads and they all end up creating their own sessionPool using the same SessionKey properties.
> Access to the if-condition needs to be synchronized so that only one session pool gets created. That will ensure that not more than the configured maximumActive number of sessions can get created.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira