RFR: 8293806: JDK_JAVA_OPTIONS picked up twice if launcher re-executes itself
Alan Bateman
alanb at openjdk.org
Thu Dec 8 14:53:45 UTC 2022
On Tue, 6 Dec 2022 14:04:44 GMT, Dmitry Samersoff <dsamersoff at openjdk.org> wrote:
> 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:
>
> https://github.com/openjdk/jdk/pull/10430
>
> This PR consists of three commits:
> 1. Cleanup of java_md.c
> 2. The implementation of early re-launch
> 3. Performance optimization
>
> @AlanBateman, @dholmes-ora Alan, David - any comments are appreciated.
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.
src/java.base/share/native/launcher/main.c line 68:
> 66: #if !defined(WINDOWS) && !defined(MACOSX)
> 67: JLI_ReExecLauncher(argc, argv);
> 68: #endif
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.
src/java.base/share/native/libjli/java.c line 368:
> 366: #endif
> 367:
> 368: JLI_SetTraceLauncher();
It's confusing to call JLI_SetTraceLauncher here as that is normally done by InitiLauncher.
src/java.base/unix/native/libjli/java_md.c line 194:
> 192: serverPatternFound = JLI_StrStr(env, serverPattern) != NULL;
> 193: minimalPatternFound = JLI_StrStr(env, serverPattern) != NULL;
> 194: if (clientPatternFound == JNI_FALSE && serverPatternFound == JNI_FALSE && minimalPatternFound == JNI_FALSE) {
This change (for minimal vm) doesn't seem to be related to re-execing.
test/jdk/tools/launcher/ArgsEnvVar.java line 227:
> 225: return;
> 226: }
> 227: env.put(JDK_JAVA_OPTIONS, "-agentlib:jdwp=transport=dt_socket,server=y,suspend=n,address=*:5005");
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.
-------------
PR: https://git.openjdk.org/jdk/pull/11538
More information about the hotspot-runtime-dev
mailing list