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
