On 21.06.2016 18:43, Aleksey Shipilev wrote:
On 06/21/2016 06:14 PM, Ivan Gerasimov wrote:
Hello!

The Pool has a member `map`, which is accessed from different threads,
thus the access is synchronized.
However, in some code paths (mostly in debug printing) it is accessed
without proper synchronization, which results in intermediate
ConcurrentModificationException when the debug output is turned on.

Would you please help review the fix?

BUGURL: https://bugs.openjdk.java.net/browse/JDK-8159822
WEBREV: http://cr.openjdk.java.net/~igerasim/8159822/00/webrev/
Missed synchronized-s:

  154     private Connections getConnections(Object id) {
  155         ConnectionsRef ref = map.get(id);
  156         return (ref != null) ? ref.getConnections() : null;
  157     }

...gets called via:

  168     public void expire(long threshold) {
  169         synchronized (map) {
       ...
  179         }
  180         expungeStaleConnections();
  181     }

...and also via:

  188     private static void expungeStaleConnections() {
...
  192             Connections conns = releaseRef.getConnections();
...
  203          }
  204     }
But this is ConnectionsWeakRef.getConnections() not Pool.getConnections(Object). As far as I can see, an instance of ConnectionsWeakRef cannot be accessed concurrently.


...

  117     public PooledConnection getPooledConnection(Object id, long
timeout,
  118         PooledConnectionFactory factory) throws NamingException {
...
             // no synchronized prior here
  127         expungeStaleConnections();
...

expungeStaleConnections() should be safe to call concurrently as long as ReferenceQueue.poll() is thread-safe.

With kind regards,
Ivan

Reply via email to