RFR: 8231584: Deadlock with ClassLoader.findLibrary and System.loadLibrary call

[Anton Kozlov]

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