RFR(xxs, jdk10): 8171508: os::jvm_path -XXaltjvm processing error after 8066474

Thu Apr 27 09:25:14 UTC 2017

Hi David,

You are right, this is more complicated.

--

Here is what I think was originally intended:

Scenario a): I start java from jdk A and give it, via -XXaltjvm, a path
pointing to a libjvm.so in jdk B. In this case, os::jvm_path() should
detect that jdk B is a full JDK, and return the path to jdk B. So, all JDK
libraries should be loaded from jdk B.

Scenario b) I start java from jdk A and give it, via -XXaltjvm, a path
pointing to a standalone libjvm.so. In this case, os::jvm_path() should see
that there is no full valid JDK around the alternate libjvm.so, and uses a
JDK from JAVA_HOME. Failing that (no JAVA_HOME set), it should use the path
from the loaded alternative libjvm.so.

--

However, there are errors:

Error 1: -XXaltjvm is currently not handled by os::jvm_path(): Arguments
<http://ld8443:8080/source/s?defs=Arguments&project=openjdk-jdk10-hs>::
sun_java_launcher_is_altjvm
<http://ld8443:8080/source/s?defs=sun_java_launcher_is_altjvm&project=openjdk-jdk10-hs>()
returns always false and we skip altjvm handling altogether. This is
because -Dsun.java.launcher.is_altjvm=true is missing. The Launcher should
set this property if XXaltjvm is set.

Side note: the gtestLauncher sets -Dsun.java.launcher.is_altjvm=true. So
starting gtests is currently the only way to hit the altjvm path in
os::jvm_path().

Side note 2: Your CDS scenario just works because the path you add to
-XXaltjvm is inside a full JDK (actually the primary JDK). This needs no
altjvm handling at all in os::jvm_path() - it just returns the path to the
primary jdk. (I wonder why you even need to specify XXaltjvm? Is this to
distinguish between server and client VM?)

--

So I do some trials with -XXaltjvm=<path>  together with explicitely
setting -Dsun.java.launcher.is_altjvm=true.

Scenario a): jdk A = ./images, jdk B = images-2

without JAVA_HOME set:  ./images/jdk/bin/java
-Dsun.java.launcher.is_altjvm=true -XXaltjvm=./images-2/jdk/lib/server

We load libjvm.so and libjava.so from images-2. Looks correct? Yes, but I
stepped thru with a debugger, what actually happens is that the
full-JDK-recognition at os_linux.cpp:2348 fails. Then it checks for
JAVA_HOME, which it is not set. Then it just gives up and falls back to the
path to the alternate libjvm.so at os_linux.cpp:2364.Which happens to be
the correct path anyway for jdk B,

Now lets set JAVA_HOME to some completely unrelated JDK. os::jvm_path()
should ignore the value and still return path to jdk b, yes? But no:

- .../output $ export
JAVA_HOME=/sapmnt/depot/tools/gen/linuxx86_64/licenseware/jse/1.5.0
- .../output $ ./images/jdk/bin/java -Dsun.java.launcher.is_altjvm=true
-XXaltjvm=./images-2/images/jdk/lib/server
Error occurred during initialization of VM
Unable to load native library:
/net/sapmnt.depot/tools/gen/linuxx86_64/licenseware/jse/1.5.0/jre/lib/libjava.so:
cannot open shared object file: No such file or directory

This is Error 2: os::jvm_path() does not recognize "images-2" to be a valid
full JDK, so it falls back to JAVA_HOME, which in this case points to some
unrelated older JDK. Here, in this case,  it fails to load the libjava.so.
But it should never have attempted to load it.

This is the bug my patch attempts to fix, admittedly you are right, there
may be more to it (the "jre" for example).

Scenario b) just works, if JAVA_HOME is set correctly. This is what the
gtestLauncher does: It sets -Dsun.java.launcher.is_altjvm=true and sets
JAVA_HOME to the value of the -jdk:<path> argument (see gtestMain.cpp).

--

So the first error is IMHO that when -XXaltjvm is passed it should set
-Dsun.java.launcher.is_altjvm=true but it does not.

The second error is that the half-baked full-jdk-detection in
os::jvm_path() does not work. You are right, there are more errors than the
5-slashes-path-traversal. This makes Scenario a) misbehave if JAVA_HOME
happens to be set to another JDK. You will either not load, as in my case,
or load JDK libraries from another JDK.

I honestly do not like this coding at all and would love to see this made
simpler. This is very difficult to understand.

Kind Regards, Thomas

On Thu, Apr 27, 2017 at 1:31 AM, David Holmes <david.holmes at oracle.com>
wrote:

> Hi Thomas,
>
> Backing up a few steps ...
>
> On 26/04/2017 5:10 PM, Thomas Stüfe wrote:
>
>> Hi all,
>>
>> may I please have a review for this tiny fix. 8066474 removed the <arch>
>> directory from the images and since then -XXaltjvm was slightly broken.
>> When handling XXaltjvm, os::jvm_path() examines the path of the libjvm.so
>> to check if it is part of what it considers a standard JDK by traversing a
>> number of slashes up the path and looking for "/jre/lib". That number of
>> slashes was off since 8066474.
>>
>
> Hold on a moment! We don't have a jre subdirectory any more either! The
> image for a jdk directly contains lib/server/libjvm.so (for example).
>
> Also jvm_path is used for dumping the shared archive using the default
> location. If I do:
>
> > ./images/jdk/bin/java -XXaltjvm=images/jdk/lib/server/ -Xshare:dump
>
> it works perfectly correctly:
>
>  > find . -name classes.jsa
> ./images/jdk/lib/server/classes.jsa
>
> So where's the bug ??
>
> David
> -----
>
>
>
> webrev:
>> http://cr.openjdk.java.net/~stuefe/webrevs/8171508-os_jvm_pa
>> th_xxaltjvm_processing_error_after_8066474/jdk10-webrev.00/
>> webrev/index.html
>>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8171508
>>
>> Note that this only affects cases where the alternate libjvm.so is part of
>> a full jdk, so it does not affect the gtestLauncher.
>>
>> Thanks & Regards, Thomas
>>
>>