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

Daniel Fuchs daniel.fuchs at oracle.com
Fri Dec 16 10:46:48 UTC 2016


Hi Rob,

Looks good to me too now.

-- daniel

On 16/12/16 03:21, Vyom Tewari wrote:
> 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
>>>>
>



More information about the core-libs-dev mailing list