[jdk9] RFR: 8159822: Non-synchronized access to shared members of com.sun.jndi.ldap.pool.Pool

[Ivan Gerasimov]

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