8314491: Linux: jexec launched via PATH fails to find java

[vpa1977]

---

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

Issue

• JDK-8314491: Linux: jexec launched via PATH fails to find java (Bug - P4)

Reviewers

• David Holmes (@dholmes-ora - Reviewer)
• Roger Riggs (@RogerRiggs - Reviewer)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 15343

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

Using diff file

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

Webrev

Link to Webrev Comment

[vpa1977]

When jexec is found in the PATH, e.g. we run $jexec myjar.jar, the argv[0] contains jexec and Java can not be found.

Running updated test without fix in jexec.c:

$make test CONF=linux-x86_64-server-slowdebug TEST=test/jdk/tools/launcher/Jexec.java 
.....

++++Test Output++++
  can't locate java: No such file or directory
++++Test Stack Trace++++
....
==============================
Test summary
==============================
   TEST                                              TOTAL  PASS  FAIL ERROR   
>> jtreg:test/jdk/tools/launcher/Jexec.java              1     0     1     0 <<
==============================
TEST FAILURE

Running test after fixing jexec.c:

$ make test CONF=linux-x86_64-server-slowdebug TEST=test/jdk/tools/launcher/Jexec.java 
....
==============================
Test summary
==============================
   TEST                                              TOTAL  PASS  FAIL ERROR   
   jtreg:test/jdk/tools/launcher/Jexec.java              1     1     0     0   
==============================
TEST SUCCESS

Rerun jdk_launcher tests:

$ make test CONF=linux-x86_64-server-slowdebug TEST=jdk_launcher
...
==============================
Test summary
==============================
   TEST                                              TOTAL  PASS  FAIL ERROR   
   jtreg:test/jdk:jdk_launcher                         124   124     0     0   
==============================
TEST SUCCESS

[bridgekeeper]

👋 Welcome back vpetko! 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]

@vpa1977 The following labels will be automatically applied to this pull request:

• core-libs
• hotspot-runtime

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.

[mlbridge]

Webrevs

• 06: Full - Incremental (be995fc6)
• 05: Full - Incremental (5354ceba)
• 04: Full - Incremental (c974e011)
• 03: Full - Incremental (a2e8a045)
• 02: Full - Incremental (3f832c40)
• 01: Full - Incremental (7bd4e677)
• 00: Full (958ee563)

[dholmes-ora]

I get the sense from the comment in jexec.c that it is only intended to be launched via a full path, so having it in the $PATH seems like a usage error to me.

That said this change seems relatively harmless, though I confess I do not understand how /proc/self/exe works and under what conditions reading it returns the actual binary path rather than "/proc/self/exe". ??

[vpa1977]

I get the sense from the comment in jexec.c that it is only intended to be launched via a full path, so having it in the $PATH seems like a usage error to me.

Unfortunately this executable is linked in /usr/bin and is assumed to work perfectly when run as just jexec .

That said this change seems relatively harmless, though I confess I do not understand how /proc/self/exe works and under what conditions reading it returns the actual binary path rather than "/proc/self/exe". ??

The '/proc/' filesystem contains a 'self' entry for the current process and 'exe' entry is expected to contain the symbolic link to the actual binary[1] . realpath call in getJavaPath() function translates the symbolic link into the binary path.

I do not have much expertise in the kernel development, but as far as I know the value of /proc/self/exe it is being set in Memory Descriptor structure whenever a process is created from a file[2].
Reading the symlink may return ENOENT (broken symlink) if there was no file[3].

[1] https://docs.kernel.org/filesystems/proc.html
[2] https://github.com/torvalds/linux/blob/f7757129e3dea336c407551c98f50057c22bb266/fs/exec.c#L1282
[3] https://github.com/torvalds/linux/blob/f7757129e3dea336c407551c98f50057c22bb266/fs/proc/base.c#L1730

[dholmes-ora]

realpath call in getJavaPath() function translates the symbolic link into the binary path.

Okay so a specific API makes this work as intended.

What I can't determine is whether this may have an adverse effect on anyone using an "unusual" path to jexec, that /proc/self/exe will report differently. To minimise the impact on existing users can this be implemented as a fallback if the initial attempt to locate java fails?

[vpa1977]

To minimise the impact on existing users can this be implemented as a fallback if the initial attempt to locate java fails?

Updated and re-run the affected test:

==============================
Test summary
==============================
   TEST                                              TOTAL  PASS  FAIL ERROR   
   jtreg:test/jdk/tools/launcher/Jexec.java              1     1     0     0   
==============================
TEST SUCCESS

[dholmes-ora]

Thanks for the update, I feel much more comfortable with this change now.

Please wait for a second review however.

Thanks

[openjdk]

@vpa1977 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:

8314491: Linux: jexec launched via PATH fails to find java

Reviewed-by: dholmes, rriggs

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 59 new commits pushed to the master branch:

• 969fcdb: 8314191: C2 compilation fails with "bad AD file"
• cef9fff: 8305507: Add support for grace period before AbortVMOnSafepointTimeout triggers
• ed2b467: 8315499: build using devkit on Linux ppc64le RHEL puts path to devkit into libsplashscreen
• 4b44575: 8305637: Remove Opaque1 nodes for Parse Predicates and clean up useless predicate elimination
• 8647f00: 8293850: need a largest_committed metric for each category of NMT's output
• 5a2e151: 8315548: G1: Document why VM_G1CollectForAllocation::doit() may allocate without completing a GC
• 9013b03: 8315442: Enable parallelism in vmTestbase/nsk/monitoring/stress/thread tests
• 744b397: 8312491: Update Classfile API snippets and examples
• 69c9ec9: 8314094: java/lang/ProcessHandle/InfoTest.java fails on Windows when run as user with Administrator privileges
• fe4f900: 8315088: C2: assert(wq.size() - before == EMPTY_LOOP_SIZE) failed: expect the EMPTY_LOOP_SIZE nodes of this body if empty
• ... and 49 more: https://git.openjdk.org/jdk/compare/3eac8905aee6edecbebcc12a41300d3ce176fbff...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.

As you do not have Committer status in this project an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@dholmes-ora, @RogerRiggs) but any other Committer may sponsor as well.

➡️ To flag this PR as ready for integration with the above commit message, type /integrate in a new comment. (Afterwards, your sponsor types /sponsor in a new comment to perform the integration).

[dholmes-ora]

Still good. Thanks

[AlanBateman]

I think looks okay. I'm just surprised that it has been found/used on Linux as this was originally went with a Solaris feature for doing JRE version selection, and I actually thought had been removed a long time ago.

[vpa1977]

Would it be ok to type "integrate" at this stage?

[vpa1977]

/integrate

[openjdk]

@vpa1977
Your change (at version be995fc) is now ready to be sponsored by a Committer.

[TheShermanTanker]

Is this all clear for sponsorship?

[vpa1977]

Is this all clear for sponsorship?

Yes, I have checked with @dholmes-ora

[RogerRiggs]

LGTM

[dholmes-ora]

/sponsor

[openjdk]

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

• 9a83d55: 8315373: Change VirtualThread to unmount after freezing, re-mount before thawing
• 68f6941: 8314452: Explicitly indicate inlining success/failure in PrintInlining
• b482e6d: 8315580: Remove unused java_lang_String::set_value_raw()
• 9b0da48: 8315410: Undocumented exceptions in java.text.StringCharacterIterator
• 578ded4: 8312418: Add Elements.getEnumConstantBody
• dccf670: 8306632: Add a JDK Property for specifying DTD support
• a62c48b: 8315891: java/foreign/TestLinker.java failed with "error occurred while instantiating class TestLinker: null"
• 9559e03: 8315578: PPC builds are broken after JDK-8304913
• e409d07: 8315696: SignedLoggerFinderTest.java test failed
• ab6a87e: 8314838: 3 compiler tests ignore vm flags
• ... and 112 more: https://git.openjdk.org/jdk/compare/3eac8905aee6edecbebcc12a41300d3ce176fbff...master

Your commit was automatically rebased without conflicts.

[openjdk]

@dholmes-ora @vpa1977 Pushed as commit dab1c21.

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