On 02/02/15 11:00, Paul Sandoz wrote:
On Jan 30, 2015, at 10:10 PM, Alan Bateman <alan.bate...@oracle.com> wrote:
On 30/01/2015 15:35, Chris Hegarty wrote:
:
Update webrev and spec diffs:
http://cr.openjdk.java.net/~chegar/8064924/01/
I think the javadoc looks much better now, thanks.
Minor comments in URL:
Java doc on URL constructor:
269 * <li>If the previous step fails to find a protocol handler, then the
270 * {@linkplain java.util.ServiceLoader ServiceLoader} mechanism
is used
271 * to locate a {@code URLStreamHandlerFactory} provider using the
system
"to locate {@code URLStreamHandlerFactory} providers using..."
Thanks Paul, Updated in the latest version ( see other mail ).
Using the plural reads better given what follows:
272 * class loader. The ordering that providers are located is
273 * implementation specific, and an implementation is free to
cache the
274 * located providers. A {@linkplain
java.util.ServiceConfigurationError
275 * ServiceConfigurationError}, {@code Error} or {@code
RuntimeException}
276 * thrown from the {@code createURLStreamHandler}, if
encountered, will
277 * be propagated to the calling thread. The {@code
278 * createURLStreamHandler} method of each provider, if
instantiated, is
279 * invoked, with the protocol string, until a provider returns
non-null,
280 * or all providers have been exhausted.
In getURLStreamHandler method:
1313 if (handler == null) {
1314 // Try the built-in protocol handler
1315 handler = defaultFactory.createURLStreamHandler(protocol);
1316 }
1317
1318 synchronized (streamHandlerLock) {
1319
1320 URLStreamHandler handler2 = null;
1321
1322 // Check again with hashtable just in case another
1323 // thread created a handler since we last checked
1324 handler2 = handlers.get(protocol);
1325
1326 if (handler2 != null) {
1327 return handler2;
1328 }
1329
1330 // Check with factory if another thread set a
1331 // factory since our last check
1332 if (!checkedWithFactory) {
1333 handler2 = handlerFromSettableFactory(protocol);
1334 }
1335
1336 if (!(handler2 == null || handler2 == NULL_HANDLER)) {
1337 // The handler from the factory must be given more
1338 // importance. Discard the default handler that
1339 // this thread created.
1340 handler = handler2;
1341 }
1342
1343 // Insert this handler into the hashtable
1344 if (handler != null) {
1345 handlers.put(protocol, handler);
1346 }
1347
1348 }
The code might read better if you place the stuff above the synchronized block
within it (assuming that causes no issues):
if (!checkedFactory) {
handle = handlerFromSettableFactory(protocol);
if (handle == NULL_HANDLER) handle = null;
}
if (handle == null) {
handler = defaultFactory.createURLStreamHandler(protocol);
}
if (handle != null) {
handlers.put(protocol, handler);
}
That is a possibility, but great effort has been put into this area
recently, by Peter, to attempt to keep the lookup of handlers lock free
( typically a volatile read, only requiring the lock when updating the
cache ).
We would also not want to hold the lock while performing the service
lookup, in which case we may end up with two synchronization blocks, so
as to keep the service lookup as lazy as possible. Or have I missed our
point?
-Chris.
Paul.
