Re: RFR: 8217364: Custom URLStreamHandler for jrt or file protocol can override default handler
On 2019-05-02 15:20, Seán Coffey wrote: Hi Claes, Yes - looks like a fine suggestion. http://cr.openjdk.java.net/~coffeys/webrev.8217364.03/webrev/ Looks good to me.. ;-) /Claes
Re: RFR: 8217364: Custom URLStreamHandler for jrt or file protocol can override default handler
Hi Claes,
Yes - looks like a fine suggestion.
http://cr.openjdk.java.net/~coffeys/webrev.8217364.03/webrev/
regards,
Sean.
On 02/05/2019 13:15, Claes Redestad wrote:
Hi Seán,
wouldn't it be more straightforward then to keep the logic intact and
skip the custom factory invocation in both cases if the protocol is
non-overrideable?
I.e., something like this:
diff -r 290283590646 src/java.base/share/classes/java/net/URL.java
--- a/src/java.base/share/classes/java/net/URL.java Tue Apr 30
23:47:00 2019 +0200
+++ b/src/java.base/share/classes/java/net/URL.java Thu May 02
14:10:57 2019 +0200
@@ -1403,8 +1403,9 @@
URLStreamHandlerFactory fac;
boolean checkedWithFactory = false;
+ boolean overrideableProtocol = isOverrideable(protocol);
- if (isOverrideable(protocol) &&
jdk.internal.misc.VM.isBooted()) {
+ if (overrideableProtocol && jdk.internal.misc.VM.isBooted()) {
// Use the factory (if any). Volatile read makes
// URLStreamHandlerFactory appear fully initialized to
current thread.
fac = factory;
@@ -1440,7 +1441,7 @@
// Check with factory if another thread set a
// factory since our last check
- if (!checkedWithFactory && (fac = factory) != null) {
+ if (overrideableProtocol && !checkedWithFactory && (fac =
factory) != null) {
handler2 = fac.createURLStreamHandler(protocol);
}
Thanks!
/Claes
On 2019-05-02 12:33, Seán Coffey wrote:
with webrev:
https://cr.openjdk.java.net/~coffeys/webrev.8217364.02/webrev/
regards,
Sean.
On 02/05/2019 11:00, Seán Coffey wrote:
Been thinking a bit more about this one.
Given that only initialization code will traverse through the "if
(!isOverrideable(protocol))" check, I think we can add
synchronization to eliminate any timing scenarios where the handlers
Hashtable gets populated via another thread after we make the first
handers.get call in getURLStreamHandler(String protocol)
regards,
Sean.
On 30/04/2019 18:19, Seán Coffey wrote:
Looking to correct an issue that arose during the JDK-8213942 fix.
https://bugs.openjdk.java.net/browse/JDK-8217364
https://cr.openjdk.java.net/~coffeys/webrev.8217364/webrev/
regards,
Sean.
Re: RFR: 8217364: Custom URLStreamHandler for jrt or file protocol can override default handler
Hi Seán,
wouldn't it be more straightforward then to keep the logic intact and
skip the custom factory invocation in both cases if the protocol is
non-overrideable?
I.e., something like this:
diff -r 290283590646 src/java.base/share/classes/java/net/URL.java
--- a/src/java.base/share/classes/java/net/URL.java Tue Apr 30 23:47:00
2019 +0200
+++ b/src/java.base/share/classes/java/net/URL.java Thu May 02 14:10:57
2019 +0200
@@ -1403,8 +1403,9 @@
URLStreamHandlerFactory fac;
boolean checkedWithFactory = false;
+boolean overrideableProtocol = isOverrideable(protocol);
-if (isOverrideable(protocol) && jdk.internal.misc.VM.isBooted()) {
+if (overrideableProtocol && jdk.internal.misc.VM.isBooted()) {
// Use the factory (if any). Volatile read makes
// URLStreamHandlerFactory appear fully initialized to
current thread.
fac = factory;
@@ -1440,7 +1441,7 @@
// Check with factory if another thread set a
// factory since our last check
-if (!checkedWithFactory && (fac = factory) != null) {
+if (overrideableProtocol && !checkedWithFactory && (fac =
factory) != null) {
handler2 = fac.createURLStreamHandler(protocol);
}
Thanks!
/Claes
On 2019-05-02 12:33, Seán Coffey wrote:
with webrev: https://cr.openjdk.java.net/~coffeys/webrev.8217364.02/webrev/
regards,
Sean.
On 02/05/2019 11:00, Seán Coffey wrote:
Been thinking a bit more about this one.
Given that only initialization code will traverse through the "if
(!isOverrideable(protocol))" check, I think we can add synchronization
to eliminate any timing scenarios where the handlers Hashtable gets
populated via another thread after we make the first handers.get call
in getURLStreamHandler(String protocol)
regards,
Sean.
On 30/04/2019 18:19, Seán Coffey wrote:
Looking to correct an issue that arose during the JDK-8213942 fix.
https://bugs.openjdk.java.net/browse/JDK-8217364
https://cr.openjdk.java.net/~coffeys/webrev.8217364/webrev/
regards,
Sean.
Re: RFR: 8217364: Custom URLStreamHandler for jrt or file protocol can override default handler
with webrev: https://cr.openjdk.java.net/~coffeys/webrev.8217364.02/webrev/ regards, Sean. On 02/05/2019 11:00, Seán Coffey wrote: Been thinking a bit more about this one. Given that only initialization code will traverse through the "if (!isOverrideable(protocol))" check, I think we can add synchronization to eliminate any timing scenarios where the handlers Hashtable gets populated via another thread after we make the first handers.get call in getURLStreamHandler(String protocol) regards, Sean. On 30/04/2019 18:19, Seán Coffey wrote: Looking to correct an issue that arose during the JDK-8213942 fix. https://bugs.openjdk.java.net/browse/JDK-8217364 https://cr.openjdk.java.net/~coffeys/webrev.8217364/webrev/ regards, Sean.
Re: RFR: 8217364: Custom URLStreamHandler for jrt or file protocol can override default handler
Been thinking a bit more about this one. Given that only initialization code will traverse through the "if (!isOverrideable(protocol))" check, I think we can add synchronization to eliminate any timing scenarios where the handlers Hashtable gets populated via another thread after we make the first handers.get call in getURLStreamHandler(String protocol) http://gbr10227.uk.oracle.com/html/ws/jdk-jdk/open/webrev.8217364.02/webrev/ regards, Sean. On 30/04/2019 18:19, Seán Coffey wrote: Looking to correct an issue that arose during the JDK-8213942 fix. https://bugs.openjdk.java.net/browse/JDK-8217364 https://cr.openjdk.java.net/~coffeys/webrev.8217364/webrev/ regards, Sean.
RFR: 8217364: Custom URLStreamHandler for jrt or file protocol can override default handler
Looking to correct an issue that arose during the JDK-8213942 fix. https://bugs.openjdk.java.net/browse/JDK-8217364 https://cr.openjdk.java.net/~coffeys/webrev.8217364/webrev/ regards, Sean.
