Thanks to all for the feedback. It makes sense to reduce the scope of the lock where possible.

I've updated the webrev :
http://cr.openjdk.java.net/~coffeys/webrev.8213942.v2/webrev/

Regards,
Sean.

On 20/11/18 20:59, Chris Hegarty wrote:
Sean,

On 20 Nov 2018, at 17:55, Seán Coffey <sean.cof...@oracle.com> wrote:

A race condition recently reported by the WLS team. Access to the handlers 
Hashtable and the factory should be made while holding the streamHandlerLock 
lock.

WLS team also made efforts to create a reproducer. I've modified to jtreg 
format and reduced it down further for unit testing.

https://bugs.openjdk.java.net/browse/JDK-8213942
webrev: http://cr.openjdk.java.net/~coffeys/webrev.8213942/webrev/
The issue being observed only applies to protocols where there is a
built-in handler implementation, otherwise I don't see how it can occur.
The test uses `http`, and clearly there is a built-in `http` handler.

This is all very racy, partly by design, partly not. I can see that the
changes in JDK 9 in this area altered the behaviour in such a manner
that the test which ran successfully on JDK 8, no longer runs
successfully on JDK 9.

I see you have some other comments, and I share Pavel's and Alan's
concern. I've taken a look at the JDK 8 code again [1], and maybe the
safest thing here is to try to revert back to a variant that closely
resembles that.

One thing to try is to move the `if` statement L1396 outside of the
synchronized block, and then remove the `else` on L1400. That will allow
both the handlers cache to be rechecked and the factory ( if there is
one ) to re-consulted. I believe that should resolve the issue, more
closely resemble JDK 8, and also address the comments so far ( with
minimal change ).

-Chris.

[1] 
http://hg.openjdk.java.net/jdk8/jdk8/jdk/file/687fd7c7986d/src/share/classes/java/net/URL.java



Reply via email to