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

[Xuelei Fan]

Hi,

There is a surprise in the regression test.  I made the update, and now 
the testing passed.  Here is the new webrev:
     http://cr.openjdk.java.net/~xuelei/8219991/webrev.01/

Thanks,
Xuelei

On 5/3/2019 6:45 AM, Daniel Fuchs wrote:
> 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
>>>
>