RFR 8064924: Update java.net.URL to work with modules

[Paul Sandoz]

On Jan 30, 2015, at 10:10 PM, Alan Bateman <Alan.Bateman at 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..."

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); 
  }

Paul.