Code Review Request, JDK-8219991 : New fix of the deadlock in sun.security.ssl.SSLSocketImpl

[Daniel Fuchs]

Hi Xuelei,

I agree this should fix the issue I was speaking of.
Looks good to me - but maybe you'll want a second
reviewer from the security-dev team :-)

best regards,

-- daniel

On 03/05/2019 14:03, Xuelei Fan wrote:
> Hi Daniel,
> 
> Good catch!
> 
> Here is a new webrev that's trying to address the problem.
>    http://cr.openjdk.java.net/~xuelei/8219991/webrev.01/
> 
> The isClosing field update is moving ahead, and a new filed hasDepleted 
> was added for threads competition.
> 
> The external test described in JDK-8219658 passed, and the regressing 
> testing jobs are still in progress.  I will update if there is a 
> surprise in the regression testing.
> 
> Thanks,
> Xuelei
> 
> On 5/3/2019 4:11 AM, Daniel Fuchs wrote:
>> Hi Xuelei,
>>
>> I believe there is still a small window of opportunity
>> for which `readLockedDeplete();` will never be called.
>>
>> The issue is in `deplete()`:
>>
>> If tryLock() fails to lock at line
>> 1046             if (readLock.tryLock()) {
>> then there's no guarantee that the reading
>> thread will not release the readLock before
>> the else { } statement is executed at line:
>> 1054                 isClosing = true;
>>
>> Thread 1:  enters `read`, locks `readLock`
>> Thread 2:  enters `deplete`, fails to lock
>> Thread 1:  finishes reading, find `isClosing` is false,
>>             releases `readLock`.
>> Thread 2:  `deplete` sets `isClosing` to true and returns
>>
>> then any further call to `read` will find `isClosing == true`
>> and return EOF.
>>
>> If that happens and I'm not mistaken, then
>> `readLockedDeplete();` will never be called.
>>
>> best regards,
>>
>> -- daniel
>>
>>
>>
>> On 02/05/2019 21:11, Xuelei Fan wrote:
>>> Hi,
>>>
>>> Could I get the following update reviewed?
>>>
>>>     http://cr.openjdk.java.net/~xuelei/8219991/webrev.00/
>>>
>>> The March5 test looks good, and the external test described in 
>>> JDK-8219658 passed.  No new regression test.
>>>
>>> Thanks,
>>> Xuelei
>>