Review request - 8169465: Deadlock in com.sun.jndi.ldap.pool.Connections

[Vyom Tewari]

Hi Rob,

Code changes looks good to me.

Thanks,

Vyom


On Thursday 15 December 2016 08:27 PM, Rob McKenna wrote:
> Gah, thanks Daniel. I originally worked this fix on 8u and neglected to
> remove synchronized when forward porting.
>
> I've been having a few discussions on altering how we do locking in this
> code in general and CoWArrayList is a nice idea. I'd like to leave this
> change until I get to that work though. Thanks for the suggestion.
>
> Updated webrev at: http://cr.openjdk.java.net/~robm/8169465/webrev.02/
>
>      -Rob
>
> On 14/12/16 04:58, Daniel Fuchs wrote:
>> Hi Rob,
>>
>> The expire(long) method is already synchronized, so there's no
>> need to call synchronized(this) inside, unless you forgot to
>> to remove synchronized from the method declaration?
>>
>> I wonder if 'conns' could be created as a CopyOnWriteArrayList.
>> Then maybe no synchronization would be needed? It's the first time
>> I'm looking at this file - so please accept this as a suggestion
>> for a simpler (and possibly insufficient) alternative ...
>>
>> best regards,
>>
>> -- daniel
>>
>> On 14/12/16 15:59, Rob McKenna wrote:
>>> Hi folks,
>>>
>>> Looking for a review of this change:
>>>
>>> http://cr.openjdk.java.net/~robm/8169465/webrev.01/
>>>
>>> This is necessary to fix a potential problem where recursive ldap calls
>>> can potentially cause a deadlock with the PoolCleaner thread.
>>>
>>> See: https://bugs.openjdk.java.net/browse/JDK-8169465
>>>
>>>     -Rob
>>>