RFD: What do we do about programs which set sys_paths to null?

[Andrew Haley]

[ People who know all this stuff already, please feel free to skip
this introduction. ]

This is a request for discussion about support for some programs that
do something illegal, not at all supported. I'd like to make this work
as intended by users, even though it's unsupported. Which seems odd,
but I still think we should do it.

There is a long-established hack (going back about 20 years) which
allows Java programs to set the property "java.library.path"
dynamically. You might need to do this if you're a Java program which
extracts a shared library from a jarfile, puts it in a tmpdir
somewhere, then loads that library.

People discovered that even though you can set "java.library.path" in
a program, it doesn't work as intended because the property is only
read once at boot time, and it is then cached in the static field
ClassLoader.usr_paths. So, people discovered that if they set
"java.library.path" then set set ClassLoader.sys_paths to null, then
called ClassLoader.loadLibrary, it works. Something like this:

   Class loaderClass = ClassLoader.class;
   Field userPaths = loaderClass.getDeclaredField( "sys_paths" );
   userPaths.setAccessible( true );
   userPaths.set( null, null );

See the answer here, for example:
https://community.oracle.com/thread/1551225?start=15&tstart=0

This worked because ClassLoader.loadLibrary() did this:

        if (sys_paths == null) {
            usr_paths = initializePath("java.library.path");
            sys_paths = initializePath("sun.boot.library.path");
        }

The paths got re-initialized.

The back-ported fix for "8231584: Deadlock with
ClassLoader.findLibrary and System.loadLibrary call", broke this
hack. The code that lazily re-initializes usr_paths and sys_paths has
gone. However, there was another important change:
ClassLoader.loadLibrary0(), which calls ClassLoader.loadLibrary, is no
longer synchronized, so there may now be concurrent accesses to
ClassLoader.loadLibrary().

[ The proposed fix. ]

I'd like to add this patch to 8u and 11u:

--- a/src/java.base/share/classes/java/lang/ClassLoader.java    Mon Feb 03 09:39:39 2020 +0100
+++ b/src/java.base/share/classes/java/lang/ClassLoader.java    Tue Mar 17 16:24:10 2020 +0000
@@ -2562,7 +2562,7 @@

     // The paths searched for libraries
     private static String usr_paths[];
-    private static String sys_paths[];
+    private static volatile String sys_paths[];

     private static String[] initializePath(String propName) {
         String ldPath = System.getProperty(propName, "");
@@ -2620,6 +2620,10 @@
                             boolean isAbsolute) {
         ClassLoader loader =
             (fromClass == null) ? null : fromClass.getClassLoader();
+        if (sys_paths == null) {
+            usr_paths = initializePath("java.library.path");
+            sys_paths = initializePath("sun.boot.library.path");
+        }
         assert sys_paths != null : "should be initialized at this point";
         assert usr_paths != null : "should be initialized at this point";

This patch does nothing unless sys_paths has been set to null. There
is some slight slowdown because a non-volatile read of sys_paths is
now volatile, but it's IMO insignificant and in any case it's less
than the overhead of the synchronized loadLibrary0().

I do not believe that this introduces a race when the user sets
sys_paths to null because

   setProperty("java.library.path") happens before (write volatile sys_paths)  (Program order)
   (write volatile sys_paths) synchronizes with (read volatile sys_paths)
   (read volatile sys_paths) happens before getProperty("java.lbrary.path") (Program order)

If the user sets sys_paths to null before setting "java.library.path"
there is indeed a race and their program might not work, but there
always was a race anyway.

I think we should do this for 8u and 11u. My justification is that even
though this is an ugly hack, it satisfies a real need and we don't want
to break users' programs in an update.

What do you think?

-- 
Andrew Haley  (he/him)
Java Platform Lead Engineer
Red Hat UK Ltd. <https://www.redhat.com>
https://keybase.io/andrewhaley
EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671