Hi Mandy, On 02.10.2019 01:08, Anton Kozlov wrote:> > > On 30.09.2019 21:18, Mandy Chung wrote: >> Skimming through the implementation, it seems to me that the >> Runtime::loadLibrary0 does not need to be synchronized. >> ClassLoader::loadLibrary0 should ensure that a native library of a given >> name is loaded and registered only once. > > Interesting. I get confused at first by a comment nearby [1]. At first sight, > it assumes that Runtime.loadLibrary0 is synchronized. But it's guarded by a > monitor of loadedLibraryNames. > > May be this will be simpler approach.
I cannot find a situation when this will be wrong. All the path from Runtime.loadLibrary (and Runtime.load) to NativeLibrary.loadLibrary do not need to be synchronized. Except for ClassLoader.sys_paths and usr_paths static fields initialization, they should be synchronized. I looked at older releases, OpenJDK 7, 8. Relevant code is pretty much the same: Runtime.{load,loadLibrary} are synced [1], ClassLoader.loadLibrary eventually grabs a lock [2] and do bookkeeping for loading a native library only once. So, if the Runtime.loadLibrary synchronization is redundant for JDK14, then it's so for older jdks, from the very beginning of _Open_JDK. I'd wish someone commented why is it there. Is it redundant since then. I've updated the webrev with this approach and changed the comment. Link is at the bottom. [1]: https://hg.openjdk.java.net/jdk6/jdk6/jdk/file/jdk6-b00/src/share/classes/java/lang/Runtime.java#l779 [2]: https://hg.openjdk.java.net/jdk6/jdk6/jdk/file/jdk6-b00/src/share/classes/java/lang/ClassLoader.java#l1732 On 03.10.2019 01:59, Mandy Chung wrote: > Just on the test, instead of storing Target and Target2 classfile bytes in > the source, the test can use jdk.test.lib.compiler.CompilerUtils Thanks for pointing that out, I've changed the test too. Updated webrev: http://cr.openjdk.java.net/~akozlov/8231584/webrev.02/ Thanks, Anton