8334433: jshell.exe runs an executable test.exe on startup

[kurashige23]

This is a backport of JDK-8334433 : jshell.exe runs an executable test.exe on startup

This bug began occurring in jdk21 due to the backport of JDK-8327476 (it likely also occurs in 17 and 11).

Original patch does not apply cleanly, there are the following differences:

・ExecTerminalProvider.java
Because of the presence of FfmTerminalProvider in the original patch, isPosixSystemStream is not executed on Windows, so test.exe is not executed. However, in jdk21, there is no FfmTerminalProvider, so isPosixSystemStream is executed and if the PATH contains the path of the directory containing unintended test.exe, this test.exe will be executed. To avoid this, I fixed to execute just isWindowsSystemStream on Windows (not including Cygwin and MSYS).

・OSUtils.java
The original patch hardcoded the paths where test command is located in the Posix environment, but these paths are virtual, so if you are using Cygwin or MSYS, you must specify a Windows path (Usually C:\cygwin64\bin or C:\msys64\usr\bin), otherwise isExecutable will not be true, and eventually isPosixSystemStream will not be able to execute the intended test.exe. To solve this problem, Cygwin or MSYS uses cygpath to specify the actual path on Windows. (In the original fix, isPosixSystemStream is not executed on Cygwin or MSYS, so this does not matter.)

・ConsoleIOContext.java
Since there is no FfmTerminalProvider in jdk21, the original fix is not applicable. Therefore, no fix was made for ConsoleIOContext.java.

・TerminalNoExecTest.java
The original fix confirms that no processes are created, but since there is no FfmTerminalProvider in jdk21, the test command is executed.
In jdk21, this fix causes the intended test command to run. Therefore, I created a test TerminalExecTest.sh and TerminalExecTest.java to verify that the intended executable `test` was executed. (The reason for creating TerminalExecTest.sh is commented within it. To put it simply, I created it due to environmental variable considerations.)

Testing:
・test/jdk/jdk/internal/jline, :langtools_jshell and test/jdk/sun/tools/jhsdb/JShellHeapDumpTest.java on RHEL9.4 and Windows Server 2022(Cygwin and MSYS), GHA testing
・I manually verified that the problem described in JDK-8334433's Description does not reproduce in Windows Command Prompt.

Thanks.

---

Progress

• JDK-8334433 needs maintainer approval
• Change must not contain extraneous whitespace
• Commit message must refer to an issue
• Change must be properly reviewed (2 reviews required, with at least 1 Reviewer, 1 Author)

Issue

• JDK-8334433: jshell.exe runs an executable test.exe on startup (Bug - P3)

Reviewers

• Paul Hohensee (@phohensee - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk21u-dev.git pull/2362/head:pull/2362
$ git checkout pull/2362

Update a local copy of the PR:
$ git checkout pull/2362
$ git pull https://git.openjdk.org/jdk21u-dev.git pull/2362/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 2362

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk21u-dev/pull/2362.diff

Using Webrev

Link to Webrev Comment

[bridgekeeper]

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

❗ This change is not yet ready to be integrated.
See the Progress checklist in the description for automated requirements.

[openjdk]

This backport pull request has now been updated with issue from the original commit.

[mlbridge]

Webrevs

• 00: Full (44b90e04)

[kurashige23]

Could anyoen revew this PR?

[kurashige23]

Could anyoen revew this PR?

[phohensee]

I'm not an expert in this area, and the patch amounts to new code, but it looks reasonable. You mention that FfmTerminalProvider doesn't exist in 21u, and that seems to motivate much of the new code. Perhaps backporting that first would make this backport straightforward.

[kurashige23]

Thank you for your comment!
FfmTerminalProvider was added in JDK-8327476. JDK-8327476 was backported to jdk21, but since the FFM API does not exist in jdk21, FfmTerminalProvider was not added to jdk21. Therefore, I made changes which does not use FfmTerminalProvider in this PR.

[phohensee]

Thanks for the explanation. Lgtm.

[openjdk]

⚠️ @kurashige23 This change is now ready for you to apply for maintainer approval. This can be done directly in each associated issue or by using the /approval command.

[kurashige23]

/approval request Backporting this patch resolves the problem that unintended executable 'test' is executed.
This backport includes bug fix and test adding.
Risk is low: only affects check process on jshell startup
Testing:
・test/jdk/jdk/internal/jline, :langtools_jshell and test/jdk/sun/tools/jhsdb/JShellHeapDumpTest.java on RHEL9.4 and Windows Server 2022(Cygwin and MSYS), GHA testing
・I manually verified that the problem described in JDK-8334433's Description does not reproduce in Windows Command Prompt.
Original patch doesn't apply cleanly because FFM is not implemented in jdk21. So I adjusted to solve the problem without using FFM.

[openjdk]

@kurashige23
8334433: The approval request has been created successfully.

[kurashige23]

@GoeLin

Sorry for sudden mention, if you have time, could you check this PR? Thanks.

[GoeLin]

Hi @kurashige23
I leave it to @jerboaa to aprove this.

[jerboaa]

/reviewers 2

Please get a second review from a jshell expert on this.

[openjdk]

@jerboaa
The total number of required reviews for this PR (including the jcheck configuration and the last /reviewers command) is now set to 2 (with at least 1 Reviewer, 1 Author).

[kurashige23]

Hi @lahodaj

Sorry for sudden mention, could you check this PR?
I think you are a jshell expert because you have made many fixes for jshell.
If you could check it when you have time, that would be great. Thanks.