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

[Xuelei Fan]

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
>