8171508: Remove -Dsun.java.launcher.is_altjvm option #24310

calvinccheung · 2025-03-29T04:28:49Z

The -Dsun.java.launcher.is_altjvm option is only used in the gtest launcher and has no relationship with the similarly named -XXaltjvm option. The gtest launcher also sets the -XX:+ExecutingUnitTests option. This RFE involves removing the -Dsun.java.launcher.is_altjvm option, capturing the -XX:+ExecutingUnitTests option in Arguments::process_sun_java_launcher_properties() so that the os::jvm_path() can check if the option is set before using JAVA_HOME setting.

Passed tiers 1 - 3 testing.

Progress

Change must be properly reviewed (1 review required, with at least 1 Reviewer)
Change must not contain extraneous whitespace
Commit message must refer to an issue
Change requires CSR request JDK-8353678 to be approved

Issues

JDK-8171508: Remove -Dsun.java.launcher.is_altjvm option (Bug - P4)
JDK-8353678: Remove the -Dsun.java.launcher.is_altjvm option (CSR)

Reviewers

David Holmes (@dholmes-ora - Reviewer)
Thomas Stuefe (@tstuefe - Reviewer) Review applies to e3ed1a2b

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/24310/head:pull/24310
$ git checkout pull/24310

Update a local copy of the PR:
$ git checkout pull/24310
$ git pull https://git.openjdk.org/jdk.git pull/24310/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 24310

View PR using the GUI difftool:
$ git pr show -t 24310

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/24310.diff

Using Webrev

Link to Webrev Comment

…m option after 8066474

bridgekeeper · 2025-03-29T04:29:51Z

👋 Welcome back ccheung! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2025-03-29T04:30:06Z

@calvinccheung This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8171508: Remove -Dsun.java.launcher.is_altjvm option

Reviewed-by: dholmes, stuefe

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 178 new commits pushed to the master branch:

36e11b6: 8353683: [REDO] j.u.l.Handler classes create deadlock risk via synchronized publish() method
0e223f1: 8349534: Refactor jdk/sun/security/krb5/runNameEquals.sh to java test
2ba80d2: 8353586: Open source several toolkit tests
... and 175 more: https://git.openjdk.org/jdk/compare/6970cf6ac69864e7027138746361e7da1983c24d...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

calvinccheung · 2025-03-29T04:32:01Z

/label add hotspot-runtime

openjdk · 2025-03-29T04:53:27Z

@calvinccheung
The hotspot-runtime label was successfully added.

mlbridge · 2025-03-29T04:56:36Z

Webrevs

tstuefe · 2025-03-29T06:25:34Z

Hi @calvinccheung, I thought we planned on removing altjvm handling altogether? See mail thread https://mail.openjdk.org/pipermail/hotspot-runtime-dev/2017-April/023205.html

tstuefe · 2025-03-29T06:38:16Z

See https://mail.openjdk.org/pipermail/hotspot-runtime-dev/2017-April/023228.html

dholmes-ora · 2025-04-01T07:09:49Z

go back 4 instead of 5 slashes and then checking for "/jre/lib";

But we got rid of the jre directory years ago. ??

calvinccheung · 2025-04-03T04:52:18Z

Hi @calvinccheung, I thought we planned on removing altjvm handling altogether? See mail thread https://mail.openjdk.org/pipermail/hotspot-runtime-dev/2017-April/023205.html

I've pushed a commit which removes altjvm handling in JVM. There's small piece of code in the java launcher (java.c) which I didn't remove because:

there's an internal test which uses altjvm;
JVM won't start if -XXaltjvm is specified

calvinccheung · 2025-04-03T04:53:36Z

go back 4 instead of 5 slashes and then checking for "/jre/lib";

But we got rid of the jre directory years ago. ??

I've removed the processing regarding the slashes and the jre directory.

src/hotspot/os/linux/os_linux.cpp

dholmes-ora · 2025-04-03T07:20:34Z

This issue should be renamed "Remove -Dsun.java.launcher.is_altjvm option". That requires a CSR request to be filed.

Once again for the record -XXaltjvm has nothing to do with the -Dsun.java.launcher.is_altjvm option.

https://mail.openjdk.org/pipermail/hotspot-runtime-dev/2017-April/023241.html

Removal of -XXaltjvm would also need a CSR request.

calvinccheung · 2025-04-03T22:36:19Z

This issue should be renamed "Remove -Dsun.java.launcher.is_altjvm option". That requires a CSR request to be filed.

I filed a draft CSR JDK-8353678.

dholmes-ora · 2025-04-06T22:05:31Z

How does the gtest launcher work with these changes? There the libjvm.so is in a completely different place to the JDK. ??

dholmes-ora · 2025-04-06T22:08:42Z

Also as discussed offline picking up JAVA_HOME from the environment is not the right thing to do in general. It only worked with -Dsun.java.launcher.is_altjvm because we ensured we set JAVA_HOME when using the property.

calvinccheung · 2025-04-06T23:55:27Z

How does the gtest launcher work with these changes? There the libjvm.so is in a completely different place to the JDK. ??

gtest launcher sets the -XX:+ExecutingUnitTests option. My updated fix is to capture the option in Arguments::process_sun_java_launcher_properties() so that the os::jvm_path() can check if the option is set before using JAVA_HOME setting.

dholmes-ora · 2025-04-07T04:48:04Z

Can you update the PR description to clarify what is now being done please Calvin.

dholmes-ora

So basically now we have removed the -Dsun.java.launcher.is_altjvm option and replaced its use with gtest by the use of the executing_unit_tests functionality. Okay.

A few minor requests. Thanks.

src/hotspot/os/aix/os_aix.cpp

src/hotspot/os/linux/os_linux.cpp

src/hotspot/os/aix/os_aix.cpp

calvinccheung · 2025-04-07T17:46:23Z

Can you update the PR description to clarify what is now being done please Calvin.

Updated PR description.

tstuefe

Are the Posix variants all identical? If yes, can we move this to os::posix.cpp? If not, what are the differences?

dholmes-ora · 2025-04-08T07:52:19Z

Are the Posix variants all identical? If yes, can we move this to os::posix.cpp? If not, what are the differences?

AIX has a different dll lookup mechanism, but otherwise these should be pretty much identical. Windows also only really differs in the lookup mechanism. Maybe new RFE to refactor these and share some code?

tstuefe · 2025-04-08T08:16:27Z

Are the Posix variants all identical? If yes, can we move this to os::posix.cpp? If not, what are the differences?

AIX has a different dll lookup mechanism, but otherwise these should be pretty much identical. Windows also only really differs in the lookup mechanism. Maybe new RFE to refactor these and share some code?

Okay. The code could also be simpler if it were to use stringStream, spanned over the caller-provided buffer. No character counting and truncation handling needed then (but we would need to add truncation detection to stringStream, but that is really easy).

calvinccheung · 2025-04-09T06:17:03Z

Are the Posix variants all identical? If yes, can we move this to os::posix.cpp? If not, what are the differences?

AIX has a different dll lookup mechanism, but otherwise these should be pretty much identical. Windows also only really differs in the lookup mechanism. Maybe new RFE to refactor these and share some code?

Okay. The code could also be simpler if it were to use stringStream, spanned over the caller-provided buffer. No character counting and truncation handling needed then (but we would need to add truncation detection to stringStream, but that is really easy).

I agree with David about doing the refactoring in a new RFE. I've simplified the code by using stringStream as you suggested. I didn't add truncation detection to stringStream (which I think could be done in another RFE). However, I added an assert in os::jvm_path() to ensure there's no truncation.

tstuefe

Nice, this looks good to me. Thank you.

dholmes-ora

Looks okay. Some additional possible cleanup now you are using the stringStream.

src/hotspot/os/aix/os_aix.cpp

src/hotspot/os/bsd/os_bsd.cpp

dholmes-ora

Looks good. Thanks

calvinccheung · 2025-04-10T19:56:28Z

Thanks @tstuefe, @dholmes-ora for the review.

/integrate

openjdk · 2025-04-10T19:57:30Z

Going to push as commit 7680f70.
Since your change was applied there have been 183 commits pushed to the master branch:

3131dd1: 8346465: Add a check in setData() to restrict the update of Built-In ICC_Profiles
af5db51: 8353641: Deprecate core library permission classes for removal
60f3d60: 8351110: ImageIO.write for JPEG can write corrupt JPEG for certain thumbnail dimensions
... and 180 more: https://git.openjdk.org/jdk/compare/6970cf6ac69864e7027138746361e7da1983c24d...master

Your commit was automatically rebased without conflicts.

openjdk · 2025-04-10T19:57:39Z

@calvinccheung Pushed as commit 7680f70.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

8171508: os::jvm_path: error in handling -Dsun.java.launcher.is_altjv…

04f09cc

…m option after 8066474

calvinccheung marked this pull request as ready for review March 29, 2025 04:29

openjdk bot added rfr Pull request is ready for review hotspot-runtime [email protected] labels Mar 29, 2025

calvinccheung added 2 commits April 2, 2025 17:45

Merge branch 'master' into 8171508-os-jvm_path

472dad8

@tstuefe and @dholmes-ora comments

eb20233

dholmes-ora reviewed Apr 3, 2025

View reviewed changes

src/hotspot/os/linux/os_linux.cpp Outdated Show resolved Hide resolved

openjdk bot added the csr Pull request needs approved CSR before integration label Apr 3, 2025

calvinccheung changed the title ~~8171508: os::jvm_path: error in handling -Dsun.java.launcher.is_altjvm option after 8066474~~ 8171508: Remove -Dsun.java.launcher.is_altjvm option Apr 3, 2025

@dholmes-ora comment

33a7ad2

fix issues related to JAVA_HOME and gtest

2de1d3a

dholmes-ora suggested changes Apr 7, 2025

View reviewed changes

@dholmes-ora comments

e107e56

tstuefe reviewed Apr 8, 2025

View reviewed changes

simplify the code by using stringStream

e3ed1a2

tstuefe approved these changes Apr 9, 2025

View reviewed changes

dholmes-ora reviewed Apr 10, 2025

View reviewed changes

src/hotspot/os/aix/os_aix.cpp Outdated Show resolved Hide resolved

src/hotspot/os/bsd/os_bsd.cpp Outdated Show resolved Hide resolved

cleanup

60a1038

dholmes-ora approved these changes Apr 10, 2025

View reviewed changes

openjdk bot added ready Pull request is ready to be integrated and removed csr Pull request needs approved CSR before integration labels Apr 10, 2025

openjdk bot added the integrated Pull request has been integrated label Apr 10, 2025

openjdk bot closed this Apr 10, 2025

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Apr 10, 2025

calvinccheung deleted the 8171508-os-jvm_path branch April 17, 2025 15:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

8171508: Remove -Dsun.java.launcher.is_altjvm option #24310

8171508: Remove -Dsun.java.launcher.is_altjvm option #24310

calvinccheung commented Mar 29, 2025 •

edited by openjdk bot

Loading

bridgekeeper bot commented Mar 29, 2025

openjdk bot commented Mar 29, 2025 •

edited

Loading

calvinccheung commented Mar 29, 2025

openjdk bot commented Mar 29, 2025

mlbridge bot commented Mar 29, 2025 •

edited

Loading

tstuefe commented Mar 29, 2025

tstuefe commented Mar 29, 2025

dholmes-ora commented Apr 1, 2025

calvinccheung commented Apr 3, 2025

calvinccheung commented Apr 3, 2025

dholmes-ora commented Apr 3, 2025

calvinccheung commented Apr 3, 2025

dholmes-ora commented Apr 6, 2025

dholmes-ora commented Apr 6, 2025

calvinccheung commented Apr 6, 2025

dholmes-ora commented Apr 7, 2025

dholmes-ora left a comment

calvinccheung commented Apr 7, 2025

tstuefe left a comment

dholmes-ora commented Apr 8, 2025

tstuefe commented Apr 8, 2025

calvinccheung commented Apr 9, 2025

tstuefe left a comment

dholmes-ora left a comment •

edited

Loading

dholmes-ora left a comment

calvinccheung commented Apr 10, 2025

openjdk bot commented Apr 10, 2025

openjdk bot commented Apr 10, 2025

8171508: Remove -Dsun.java.launcher.is_altjvm option #24310

8171508: Remove -Dsun.java.launcher.is_altjvm option #24310

Conversation

calvinccheung commented Mar 29, 2025 • edited by openjdk bot Loading

Progress

Issues

Reviewers

Reviewing

bridgekeeper bot commented Mar 29, 2025

openjdk bot commented Mar 29, 2025 • edited Loading

calvinccheung commented Mar 29, 2025

openjdk bot commented Mar 29, 2025

mlbridge bot commented Mar 29, 2025 • edited Loading

Webrevs

tstuefe commented Mar 29, 2025

tstuefe commented Mar 29, 2025

dholmes-ora commented Apr 1, 2025

calvinccheung commented Apr 3, 2025

calvinccheung commented Apr 3, 2025

dholmes-ora commented Apr 3, 2025

calvinccheung commented Apr 3, 2025

dholmes-ora commented Apr 6, 2025

dholmes-ora commented Apr 6, 2025

calvinccheung commented Apr 6, 2025

dholmes-ora commented Apr 7, 2025

dholmes-ora left a comment

Choose a reason for hiding this comment

calvinccheung commented Apr 7, 2025

tstuefe left a comment

Choose a reason for hiding this comment

dholmes-ora commented Apr 8, 2025

tstuefe commented Apr 8, 2025

calvinccheung commented Apr 9, 2025

tstuefe left a comment

Choose a reason for hiding this comment

dholmes-ora left a comment • edited Loading

Choose a reason for hiding this comment

dholmes-ora left a comment

Choose a reason for hiding this comment

calvinccheung commented Apr 10, 2025

openjdk bot commented Apr 10, 2025

openjdk bot commented Apr 10, 2025

calvinccheung commented Mar 29, 2025 •

edited by openjdk bot

Loading

openjdk bot commented Mar 29, 2025 •

edited

Loading

mlbridge bot commented Mar 29, 2025 •

edited

Loading

dholmes-ora left a comment •

edited

Loading