[jira] [Issue Comment Edited] (AMQ-3506) Access to ConnectionPool.createSession needs to be synchronized

["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