[9] RFR(S): 8178726: Can't load classes from classpath if it is a UNC share

12 messages
Open this post in threaded view
|

 Hi, can you please review the following change which fixes a problem with UNC paths on the Windows class path: http://cr.openjdk.java.net/~simonis/webrevs/2017/8178726/https://bugs.openjdk.java.net/browse/JDK-8178726I would appreciate if somebody could run this trough JPRT for me. I won't be available until Tuesday after Easter so there's some time for testing :) Here comes the detailed problem description (also included in the bug report): If we set the classpath to a UNC share (e.g. '-cp \\foo\bar\classes') java won't be able to find classes there. This is also true if we set the classpath to a drive letter which maps to a network drive (e.g. '-cp Z:\classes' where 'Z:' is a short-cut for '\\foo\bar'). This error is clearly a regression and should be fixed before Java 9 GA. The problem is caused by the refactoring of the application class loader: Before Java 9 the application class loader was a URLClassLoader (actually sun.misc.Launcher$AppClassLoader which extends URLClassLoader). It took the classpath from the "java.class.path" property and transformed it into a File[] by calling Launcher.getClassPath(). This File[] was then converted into a URL[] by calling Launcher.pathToURLs() which in turn called Launcher.getFileURL() which finally called sun.net.www.ParseUtil.fileToEncodedURL() to convert a File into an URL. This last function explicitly creates URLs with protocol 'file' and empty authority (i.e. if the File contained an authority part like "\\foo" in "\\foo\bar\classs"), this was saved in the path part of the URL (i.e. 'return new URL("file", "", path)'); Later, these URLs were used to access classes on the classpath. This was done by creating new File objects from the path components of the corresponding URLs and because these path components already contained the full UNC path, everything worked fine. In JDK 9 now, the legacy class path is constructed in jdk.internal.loader.ClassLoaders. It also takes the "java.class.path" property as a starting point but calls addClassPathToUCP() which in turn calls toFileURL() which finally uses 'Paths.get(s).toRealPath().toUri().toURL()' to transform a string 's' into an URL. This conversion creates URLs where the authority part contains the hostname and the path component contains the remaining path from the UNC paths in the classpath. The problem is now that during class loading, the new application class loader jdk.internal.loader.ClassLoaders$AppClassLoader calls jdk.internal.loader.BuiltinClassLoader.loadClass() which calls loadClassOrNull() which calls findClassOnClassPathOrNull() which calls jdk.internal.loader.URLClassPath.getResource(). The class URLClassPath has a list of all the previously created URLs for each entry of the classpath. For each of these URLs it creates a jdk.internal.loader.URLClassPath$Loader by calling getLoader(). If the corresponding URL represents a file, it will create a jdk.internal.loader.URLClassPath$FileLoader (which derives from URLClassPath$Loader). The FileLoader class uses the file component of the URL to construct a File object for accessing the underlying ressources. But unlike the jdk8 case, the file part of the URL now only contains the path component WITHOUT the authority (i.e. 'host') part. This will be interpreted as a relative path realtively to the current drive and obviously fail. Thanks, Volker Reply | Threaded Open this post in threaded view | Re: [9] RFR(S): 8178726: Can't load classes from classpath if it is a UNC share  On 13/04/2017 19:50, Volker Simonis wrote: > Hi, > > can you please review the following change which fixes a problem with > UNC paths on the Windows class path: > > http://cr.openjdk.java.net/~simonis/webrevs/2017/8178726/> https://bugs.openjdk.java.net/browse/JDK-8178726> > I would appreciate if somebody could run this trough JPRT for me. I > won't be available until Tuesday after Easter so there's some time for > testing :) Yes, this is a regression, it seems specific to a class path with exploded classes on a network share. The issue has been there since jdk-9+111 but I don't think has been reported before now. I can do some testing with your patch. My general concern is that there are lot of conversions going on, specifically URL -> URI -> Path -> File (this is after the initial setup that amounts to String -> Path -> URI -> URL). I think URLClassPath (which dates back to JDK 1.2 I think) is crying out to be replaced and we probably should have done a long time ago. Too late now as we have to be cautious. Given that dir is only needed when the resource name contains a ".." then there shouldn't be any need to initialize it in the constructor, it can be lazy. That would reduce the risk with changing this fragile area. It would also remove some of the overhead with a really long class path. Once JDK 9 is out of the way then I think we should replace URLClassPath.FileLoader, if not all of URLClassPath. -Alan Reply | Threaded Open this post in threaded view | Re: [9] RFR(S): 8178726: Can't load classes from classpath if it is a UNC share  I see some changes: - relative url won't be supported - path is no longer canonicalized Maybe these are no problem for URLClassPath. Just my 2 cents. --Max On 04/14/2017 04:41 PM, Alan Bateman wrote: > On 13/04/2017 19:50, Volker Simonis wrote: > >> Hi, >> >> can you please review the following change which fixes a problem with >> UNC paths on the Windows class path: >> >> http://cr.openjdk.java.net/~simonis/webrevs/2017/8178726/>> https://bugs.openjdk.java.net/browse/JDK-8178726>> >> I would appreciate if somebody could run this trough JPRT for me. I >> won't be available until Tuesday after Easter so there's some time for >> testing :) > Yes, this is a regression, it seems specific to a class path with > exploded classes on a network share. The issue has been there since > jdk-9+111 but I don't think has been reported before now. > > I can do some testing with your patch. My general concern is that there > are lot of conversions going on, specifically URL -> URI -> Path -> File > (this is after the initial setup that amounts to String -> Path -> URI > -> URL). I think URLClassPath (which dates back to JDK 1.2 I think) is > crying out to be replaced and we probably should have done a long time > ago. Too late now as we have to be cautious. > > Given that dir is only needed when the resource name contains a ".." > then there shouldn't be any need to initialize it in the constructor, it > can be lazy. That would reduce the risk with changing this fragile area. > It would also remove some of the overhead with a really long class path. > Once JDK 9 is out of the way then I think we should replace > URLClassPath.FileLoader, if not all of URLClassPath. > > -Alan Reply | Threaded Open this post in threaded view | Re: [9] RFR(S): 8178726: Can't load classes from classpath if it is a UNC share  On 14/04/2017 10:02, Weijun Wang wrote: > I see some changes: > > - relative url won't be supported > > - path is no longer canonicalized > > Maybe these are no problem for URLClassPath. Just my 2 cents. The URLs are already constructed from canonicalized file paths but maybe you are thinking about usages of URLClassPath by other code? -Alan Reply | Threaded Open this post in threaded view | Re: [9] RFR(S): 8178726: Can't load classes from classpath if it is a UNC share  In reply to this post by Volker Simonis Volker, On 13/04/17 19:50, Volker Simonis wrote: > Hi, > > can you please review the following change which fixes a problem with > UNC paths on the Windows class path: > > http://cr.openjdk.java.net/~simonis/webrevs/2017/8178726/> https://bugs.openjdk.java.net/browse/JDK-8178726> > I would appreciate if somebody could run this trough JPRT for me. I > won't be available until Tuesday after Easter so there's some time for > testing :) I know that Alan has provided some comments on this, but I just grabbed your patch and sent it through our internal build and test system. I observed a few failures: 1)java/nio/file/spi/SetDefaultProvider.java ( all platforms ) STDERR: Error: A JNI error has occurred, please check your installation and try again Exception in thread "main" java.lang.Error: java.lang.NullPointerException at java.base/java.nio.file.FileSystems$DefaultFileSystemHolder.getDefaultProvider(FileSystems.java:139)         at java.base/java.nio.file.FileSystems$DefaultFileSystemHolder.access$100(FileSystems.java:100)         at java.base/java.nio.file.FileSystems$DefaultFileSystemHolder$1.run(FileSystems.java:109)         at java.base/java.nio.file.FileSystems$DefaultFileSystemHolder$1.run(FileSystems.java:107)         at java.base/java.security.AccessController.doPrivileged(Native Method)         at java.base/java.nio.file.FileSystems$DefaultFileSystemHolder.defaultFileSystem(FileSystems.java:107) at java.base/java.nio.file.FileSystems$DefaultFileSystemHolder.(FileSystems.java:101)         at java.base/java.nio.file.FileSystems.getDefault(FileSystems.java:188)         at java.base/java.nio.file.Paths.get(Paths.java:138)         at java.base/jdk.internal.loader.URLClassPath$FileLoader.(URLClassPath.java:1049) at java.base/jdk.internal.loader.URLClassPath$3.run(URLClassPath.java:417)         at java.base/jdk.internal.loader.URLClassPath$3.run(URLClassPath.java:411) at java.base/java.security.AccessController.doPrivileged(Native Method) at java.base/jdk.internal.loader.URLClassPath.getLoader(URLClassPath.java:410) at java.base/jdk.internal.loader.URLClassPath.getLoader(URLClassPath.java:379) at java.base/jdk.internal.loader.URLClassPath.getResource(URLClassPath.java:245) at java.base/jdk.internal.loader.BuiltinClassLoader.findClassOnClassPathOrNull(BuiltinClassLoader.java:666) at java.base/jdk.internal.loader.BuiltinClassLoader.loadClassOrNull(BuiltinClassLoader.java:592) at java.base/jdk.internal.loader.BuiltinClassLoader.loadClass(BuiltinClassLoader.java:550) at java.base/jdk.internal.loader.ClassLoaders$AppClassLoader.loadClass(ClassLoaders.java:185)         at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:473)         at java.base/java.lang.Class.forName0(Native Method)         at java.base/java.lang.Class.forName(Class.java:374)         at java.base/sun.launcher.LauncherHelper.loadMainClass(LauncherHelper.java:628)         at java.base/sun.launcher.LauncherHelper.checkAndLoadMain(LauncherHelper.java:524) Caused by: java.lang.NullPointerException         at java.base/java.nio.file.Paths.get(Paths.java:138)         at java.base/jdk.internal.loader.URLClassPath$FileLoader.(URLClassPath.java:1049) at java.base/jdk.internal.loader.URLClassPath$3.run(URLClassPath.java:417)         at java.base/jdk.internal.loader.URLClassPath$3.run(URLClassPath.java:411) at java.base/java.security.AccessController.doPrivileged(Native Method) at java.base/jdk.internal.loader.URLClassPath.getLoader(URLClassPath.java:410) at java.base/jdk.internal.loader.URLClassPath.getLoader(URLClassPath.java:379) at java.base/jdk.internal.loader.URLClassPath.getResource(URLClassPath.java:245) at java.base/jdk.internal.loader.BuiltinClassLoader.findClassOnClassPathOrNull(BuiltinClassLoader.java:666) at java.base/jdk.internal.loader.BuiltinClassLoader.loadClassOrNull(BuiltinClassLoader.java:592) at java.base/jdk.internal.loader.BuiltinClassLoader.loadClass(BuiltinClassLoader.java:550) at java.base/jdk.internal.loader.ClassLoaders$AppClassLoader.loadClass(ClassLoaders.java:185)         at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:473)         at java.base/java.lang.Class.forName0(Native Method)         at java.base/java.lang.Class.forName(Class.java:374)         at java.base/java.nio.file.FileSystems$DefaultFileSystemHolder.getDefaultProvider(FileSystems.java:129) ... 24 more 2) java/security/Security/ClassLoaderDeadlock/ClassLoaderDeadlock.sh ( all platforms ) Exception in thread "main" java.lang.IllegalArgumentException: URI is not hierarchical at java.base/sun.nio.fs.UnixUriUtils.fromUri(UnixUriUtils.java:48) at java.base/sun.nio.fs.UnixFileSystemProvider.getPath(UnixFileSystemProvider.java:99) at java.base/java.nio.file.Paths.get(Paths.java:138) at java.base/jdk.internal.loader.URLClassPath$FileLoader.(URLClassPath.java:1049)         at java.base/jdk.internal.loader.URLClassPath$3.run(URLClassPath.java:417) at java.base/jdk.internal.loader.URLClassPath$3.run(URLClassPath.java:411)         at java.base/java.security.AccessController.doPrivileged(Native Method)         at java.base/jdk.internal.loader.URLClassPath.getLoader(URLClassPath.java:410)         at java.base/jdk.internal.loader.URLClassPath.getLoader(URLClassPath.java:379)         at java.base/jdk.internal.loader.URLClassPath.getResource(URLClassPath.java:245)         at java.base/java.net.URLClassLoader$1.run(URLClassLoader.java:450) at java.base/java.net.URLClassLoader$1.run(URLClassLoader.java:447)         at java.base/java.security.AccessController.doPrivileged(Native Method)         at java.base/java.net.URLClassLoader.findClass(URLClassLoader.java:446)         at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:540)         at ClassLoaderDeadlock$DelayClassLoader.loadClass(ClassLoaderDeadlock.java:87) at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:473) at ClassLoaderDeadlock.main(ClassLoaderDeadlock.java:38) 3) test\java\net\URLClassLoader\closetest\CloseTest.java ( Windows ) java.lang.IllegalArgumentException: uri at java.base/jdk.internal.loader.URLClassPath$FileLoader.(URLClassPath.java:1051)         at java.base/jdk.internal.loader.URLClassPath$3.run(URLClassPath.java:417) at java.base/jdk.internal.loader.URLClassPath$3.run(URLClassPath.java:411)         at java.base/java.security.AccessController.doPrivileged(Native Method)         at java.base/jdk.internal.loader.URLClassPath.getLoader(URLClassPath.java:410)         at java.base/jdk.internal.loader.URLClassPath.getLoader(URLClassPath.java:379)         at java.base/jdk.internal.loader.URLClassPath.getResource(URLClassPath.java:245)         at java.base/java.net.URLClassLoader$1.run(URLClassLoader.java:450) at java.base/java.net.URLClassLoader$1.run(URLClassLoader.java:447)         at java.base/java.security.AccessController.doPrivileged(Native Method)         at java.base/java.net.URLClassLoader.findClass(URLClassLoader.java:446)         at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:540)         at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:473)         at java.base/java.lang.Class.forName0(Native Method)         at java.base/java.lang.Class.forName(Class.java:374)         at Common.loadClass(Common.java:80)         at CloseTest.test(CloseTest.java:109)         at CloseTest.main(CloseTest.java:83)         at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)         at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)         at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)         at java.base/java.lang.reflect.Method.invoke(Method.java:563)         at com.sun.javatest.regtest.agent.MainWrapper$MainThread.run(MainWrapper.java:115) at java.base/java.lang.Thread.run(Thread.java:844) Caused by: java.net.URISyntaxException: Illegal character in opaque part at index 7: file:C:\jprt\T\P1\101420.chhegar\s\jdk\testoutput\jdk_net\JTwork\classes\java\net\URLClassLoader\closetest/testdir/ at java.base/java.net.URI$Parser.fail(URI.java:2904)         at java.base/java.net.URI$Parser.checkChars(URI.java:3075) at java.base/java.net.URI$Parser.parse(URI.java:3111)         at java.base/java.net.URI.(URI.java:590)         at java.base/java.net.URL.toURI(URL.java:1018)         at java.base/jdk.internal.loader.URLClassPath$FileLoader.(URLClassPath.java:1049) ... 23 more 4) java/net/URLClassLoader/closetest/GetResourceAsStream.java ( Windows ) STDOUT: Doing tests with URL: file:/C:/jprt/T/P1/101420.chhegar/s/jdk/testoutput/jdk_net/JTwork/classes/java/net/URLClassLoader/closetest/test.jar ... OK Doing tests with URL: file:/C:/jprt/T/P1/101420.chhegar/s/jdk/testoutput/jdk_net/JTwork/classes/java/net/URLClassLoader/closetest/test.jar ... OK Doing tests with URL: file:/C:/jprt/T/P1/101420.chhegar/s/jdk/testoutput/jdk_net/JTwork/classes/java/net/URLClassLoader/closetest/test.jar ... OK STDERR: java.lang.RuntimeException: Dir exists: C:\jprt\T\P1\101420.chhegar\s\jdk\testoutput\jdk_net\JTwork\classes\java\net\URLClassLoader\closetest\testdir at Common.copyDir(Common.java:62) at GetResourceAsStream.main(GetResourceAsStream.java:71) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.base/java.lang.reflect.Method.invoke(Method.java:563) at com.sun.javatest.regtest.agent.MainWrapper$MainThread.run(MainWrapper.java:115)         at java.base/java.lang.Thread.run(Thread.java:844) -Chris.
Open this post in threaded view
|

Re: [9] RFR(S): 8178726: Can't load classes from classpath if it is a UNC share

 On 14/04/2017 14:02, Chris Hegarty wrote: > > I know that Alan has provided some comments on this, but I just > grabbed your patch and sent it through our internal build and test > system. I observed a few failures: Yes, I think URLClassLoader will be broken (Max is right, I forgot that URLClassPath is essentially the implementation of URLClassLoader). I think Volker will need to change this so that ClassLoaders.toFileURL(String) uses toFile().toURI().toURL(). That will ensure that URLClassPath doesn't get URLs with the server name in the authority component. -Alan
Open this post in threaded view
|

Re: [9] RFR(S): 8178726: Can't load classes from classpath if it is a UNC share

 Hi Alan, Chris, thanks for your help and feedback. It seems my fix was indeed "too optimistic". I've only briefly looked into the errors reported by Chris. The first one is due to a cyclic dependency between Paths.get() and FileSystems.getDefault(). So I've finally decided to go with Alan's proposal which is probably the least invasive one: http://cr.openjdk.java.net/~simonis/webrevs/2017/8178726.v1/It would be great if someone could verify this new version once again by running it through JPRT (I've verified that the reported problems don't occur any more, but who knows :) Thank you and best regards, Volker On Fri, Apr 14, 2017 at 3:13 PM, Alan Bateman <[hidden email]> wrote: > On 14/04/2017 14:02, Chris Hegarty wrote: > > >> I know that Alan has provided some comments on this, but I just >> grabbed your patch and sent it through our internal build and test >> system. I observed a few failures: >> > Yes, I think URLClassLoader will be broken (Max is right, I forgot that > URLClassPath is essentially the implementation of URLClassLoader). > > I think Volker will need to change this so that > ClassLoaders.toFileURL(String) uses toFile().toURI().toURL(). That will > ensure that URLClassPath doesn't get URLs with the server name in the > authority component. > > -Alan >
Open this post in threaded view
|

Re: [9] RFR(S): 8178726: Can't load classes from classpath if it is a UNC share

 On 18/04/2017 18:44, Volker Simonis wrote: > Hi Alan, Chris, > > thanks for your help and feedback. > It seems my fix was indeed "too optimistic". I've only briefly looked > into the errors reported by Chris. The first one is due to a cyclic > dependency between Paths.get() and FileSystems.getDefault(). > > So I've finally decided to go with Alan's proposal which is probably > the least invasive one: > > http://cr.openjdk.java.net/~simonis/webrevs/2017/8178726.v1/  > > > It would be great if someone could verify this new version once again > by running it through JPRT (I've verified that the reported problems > don't occur any more, but who knows :) I ran it through the build+test system and don't see any issues. It's great that you found this issue before GA, I suspect testing with a directory of .class files on a UNC isn't too common (for performance reasons) which is why it wasn't noticed (the issue has been in JDK 9 for more than a year). One additional suggestion is to include a comment, something like: // use File::toURL as URLClassPath can't handle URLs with a UNC // server name in the authority component as it won't be immediately clear to readers why the toFile() is needed in the chain. -Alan
Open this post in threaded view
|

Re: [9] RFR(S): 8178726: Can't load classes from classpath if it is a UNC share

 On Wed, Apr 19, 2017 at 9:03 AM, Alan Bateman <[hidden email]> wrote: > On 18/04/2017 18:44, Volker Simonis wrote: > > Hi Alan, Chris, > > thanks for your help and feedback. > It seems my fix was indeed "too optimistic". I've only briefly looked into > the errors reported by Chris. The first one is due to a cyclic dependency > between Paths.get() and FileSystems.getDefault(). > > So I've finally decided to go with Alan's proposal which is probably the > least invasive one: > > http://cr.openjdk.java.net/~simonis/webrevs/2017/8178726.v1/> > It would be great if someone could verify this new version once again by > running it through JPRT (I've verified that the reported problems don't > occur any more, but who knows :) > > I ran it through the build+test system and don't see any issues. It's great > that you found this issue before GA, I suspect testing with a directory of > .class files on a UNC isn't too common (for performance reasons) which is > why it wasn't noticed (the issue has been in JDK 9 for more than a year). > > One additional suggestion is to include a comment, something like: > > // use File::toURL as URLClassPath can't handle URLs with a UNC > // server name in the authority component > > as it won't be immediately clear to readers why the toFile() is needed in > the chain. That's a good suggestion. I've updated the patch with a slightly modified comment: // Use an intermediate File object to construct a URI/URL without // authority component as URLClassPath can't handle URLs with a UNC // server name in the authority component. and also updated the copyright year: http://cr.openjdk.java.net/~simonis/webrevs/2017/8178726.v2/Can you please add a "jdk9-fix-yes" label to the bug such that I can push the change? Thanks, Volker > > -Alan
Open this post in threaded view
|

Re: [9] RFR(S): 8178726: Can't load classes from classpath if it is a UNC share

 > On 19 Apr 2017, at 10:18, Volker Simonis <[hidden email]> wrote: > >> ... > > That's a good suggestion. I've updated the patch with a slightly > modified comment: > > // Use an intermediate File object to construct a URI/URL without > // authority component as URLClassPath can't handle URLs with a UNC > // server name in the authority component. > > and also updated the copyright year: > > http://cr.openjdk.java.net/~simonis/webrevs/2017/8178726.v2/The changes look good. Thanks Volker. -Chris.