Code Review Request, JDK-8219991 : New fix of the deadlock in sun.security.ssl.SSLSocketImpl
Xuelei Fan
xuelei.fan at oracle.com
Sat May 4 01:24:55 UTC 2019
Oops, forgot to update the link of the webrev. It should be:
http://cr.openjdk.java.net/~xuelei/8219991/webrev.02/
Xuelei
On 5/3/2019 6:23 PM, Xuelei Fan wrote:
> 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/
http://cr.openjdk.java.net/~xuelei/8219991/webrev.02/
>
> 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
>>>>
>>
More information about the security-dev
mailing list