8293806: JDK_JAVA_OPTIONS picked up twice if launcher re-executes itself #11538
Conversation
|
👋 Welcome back dsamersoff! A progress list of the required criteria for merging this PR into |
|
@dsamersoff The following labels will be automatically applied to this pull request:
When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing lists. If you would like to change these labels, use the /label pull request command. |
openjdk
bot
added
hotspot-runtime
|
/issue JDK-8293806 |
|
@dsamersoff This issue is referenced in the PR title - it will now be updated. |
|
I skimmed through the changes and agree there is an issue with options that shouldn't be repeated. I don't think the overall approach is too bad but I suspect it will go through a few iterations. Right now, all LD_LIBRARY_PATH handling is the "unix" java_md.c and I think we should keep it there. I wonder if the SETENV_REQUIRED removal could be split out and done first as that would remove some of the changes and make it easier to understand the impact of the changes. |
| } | ||
| #endif | ||
|
|
||
| JLI_SetTraceLauncher(); |
There was a problem hiding this comment.
It's confusing to call JLI_SetTraceLauncher here as that is normally done by InitiLauncher.
There was a problem hiding this comment.
Agree. Will file a separate CR for java_md.c cleanup.
| /* This test has no value on windows */ | ||
| return; | ||
| } | ||
| env.put(JDK_JAVA_OPTIONS, "-agentlib:jdwp=transport=dt_socket,server=y,suspend=n,address=*:5005"); |
There was a problem hiding this comment.
Tests uses hard coded ports are problematic as they will fail if the port is in used by something else, like another tests running concurrently.
| @@ -194,7 +190,8 @@ ContainsLibJVM(const char *env) { | |||
| /* to optimize for time, test if any of our usual suspects are present. */ | |||
| clientPatternFound = JLI_StrStr(env, clientPattern) != NULL; | |||
| serverPatternFound = JLI_StrStr(env, serverPattern) != NULL; | |||
| if (clientPatternFound == JNI_FALSE && serverPatternFound == JNI_FALSE) { | |||
| minimalPatternFound = JLI_StrStr(env, serverPattern) != NULL; | |||
| if (clientPatternFound == JNI_FALSE && serverPatternFound == JNI_FALSE && minimalPatternFound == JNI_FALSE) { | |||
There was a problem hiding this comment.
This change (for minimal vm) doesn't seem to be related to re-execing.
|
|
||
| #if !defined(WINDOWS) && !defined(MACOSX) | ||
| JLI_ReExecLauncher(argc, argv); | ||
| #endif |
There was a problem hiding this comment.
The naming is problematic here as it looks like it always re-execs on Unix/Linux systems. I think what you are looking for is something like ReExecLauncherIfNeeded or something that make it clear that it doesn't re-exec always.
|
@dsamersoff This pull request has been inactive for more than 8 weeks and will be automatically closed if another 8 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration! |
|
/open |
|
@dsamersoff This pull request is already open |
|
@dsamersoff This pull request has been inactive for more than 8 weeks and will be automatically closed if another 8 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration! |
|
@dsamersoff This pull request has been inactive for more than 16 weeks and will now be automatically closed. If you would like to continue working on this pull request in the future, feel free to reopen it! This can be done using the |
If the user has set LD_LIBRARY_PATH to use a particular libjvm.so, options from the JDK_JAVA_OPTIONS environment variable are picked up twice.
If an option cannot be accepted twice (e.g., -agentlib), the application fails to start.
The same happens on operating systems that doesn't support $RPATH/$ORIGIN and always have to set LD_LIBRARY_PATH and re-exec the launcher.
This fix takes the approach to re-launch as early as possible, as discussed here:
#10430
This PR consists of three commits:
@AlanBateman, @dholmes-ora Alan, David - any comments are appreciated.
Progress
Issue
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/11538/head:pull/11538$ git checkout pull/11538Update a local copy of the PR:
$ git checkout pull/11538$ git pull https://git.openjdk.org/jdk.git pull/11538/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 11538View PR using the GUI difftool:
$ git pr show -t 11538Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/11538.diff
Webrev
Link to Webrev Comment