Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

8325567: jspawnhelper without args fails with segfault #18074

Closed
wants to merge 3 commits into from

Conversation

vpa1977
Copy link
Contributor

@vpa1977 vpa1977 commented Mar 1, 2024

This MR fixes segsegv in jspawnhelper when it is called without args.
This scenario happens when a long running Java process is not restarted during upgrade.

It updates test/jdk/java/lang/ProcessBuilder/JspawnhelperProtocol.java to check that jspawnhelper exits with code 1:

After test update:

$ make CONF=linux-x86_64-server-fastdebug test TEST=test/jdk/java/lang/ProcessBuilder/JspawnhelperProtocol.java
...
Running jspawnhelper without args
STDERR:
java.lang.Exception: Parent process exited with 12
        at JspawnhelperProtocol.simulateJspawnhelperWithoutArgs(JspawnhelperProtocol.java:126)
        at JspawnhelperProtocol.main(JspawnhelperProtocol.java:267)
        at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
        at java.base/java.lang.reflect.Method.invoke(Method.java:580)
        at com.sun.javatest.regtest.agent.MainWrapper$MainTask.run(MainWrapper.java:138)
        at java.base/java.lang.Thread.run(Thread.java:1575)
...
==============================
Test summary
==============================
   TEST                                              TOTAL  PASS  FAIL ERROR   
   jtreg:test/jdk/java/lang/ProcessBuilder/JspawnhelperProtocol.java
>>                                                       1     0     1     0 <<
==============================
TEST FAILURE

After jspawnhelper change the test passes:

==============================
Test summary
==============================
   TEST                                              TOTAL  PASS  FAIL ERROR   
   jtreg:test/jdk/java/lang/ProcessBuilder/JspawnhelperProtocol.java
                                                         1     1     0     0   
==============================
TEST SUCCESS

The user will see the following output in the logs:

An earlier version of Java is trying to call jspawnhelper.
Please restart Java process.
Exception in thread "main" java.io.IOException: Cannot run program "ls": error=0, Failed to exec spawn helper: pid: 2168121, exit value: 1
        at java.base/java.lang.ProcessBuilder.start(ProcessBuilder.java:1143)
        at java.base/java.lang.ProcessBuilder.start(ProcessBuilder.java:1073)
        at Test.main(Test.java:3)
Caused by: java.io.IOException: error=0, Failed to exec spawn helper: pid: 2168121, exit value: 1
        at java.base/java.lang.ProcessImpl.forkAndExec(Native Method)
        at java.base/java.lang.ProcessImpl.<init>(ProcessImpl.java:314)
        at java.base/java.lang.ProcessImpl.start(ProcessImpl.java:244)
        at java.base/java.lang.ProcessBuilder.start(ProcessBuilder.java:1110)

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-8325567: jspawnhelper without args fails with segfault (Bug - P4)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 18074

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

Using diff file

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

Webrev

Link to Webrev Comment

@bridgekeeper
Copy link

bridgekeeper bot commented Mar 1, 2024

👋 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
Copy link

openjdk bot commented Mar 1, 2024

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

  • core-libs

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

@openjdk openjdk bot added the core-libs core-libs-dev@openjdk.org label Mar 1, 2024
@vpa1977
Copy link
Contributor Author

vpa1977 commented Mar 1, 2024

Manual test:

public class Test {
    public static void main(String[] args) throws Throwable {
        Process p = new ProcessBuilder("ls", "-alrt", "/tmp").start();
        p.waitFor();
    }
}

Running older java with a new jspawnhelper will result in the following output:

java --version
openjdk 17.0.9-internal 2023-10-17
OpenJDK Runtime Environment (build 17.0.9-internal+0-adhoc.vladimirp.jdk17u)
OpenJDK 64-Bit Server VM (build 17.0.9-internal+0-adhoc.vladimirp.jdk17u, mixed mode)

$ ./java -cp . Test
An earlier version of Java is trying to call jspawnhelper.
Please restart Java process.
Exception in thread "main" java.io.IOException: Cannot run program "ls": error=0, Failed to exec spawn helper: pid: 2168121, exit value: 1
        at java.base/java.lang.ProcessBuilder.start(ProcessBuilder.java:1143)
        at java.base/java.lang.ProcessBuilder.start(ProcessBuilder.java:1073)
        at Test.main(Test.java:3)
Caused by: java.io.IOException: error=0, Failed to exec spawn helper: pid: 2168121, exit value: 1
        at java.base/java.lang.ProcessImpl.forkAndExec(Native Method)
        at java.base/java.lang.ProcessImpl.<init>(ProcessImpl.java:314)
        at java.base/java.lang.ProcessImpl.start(ProcessImpl.java:244)
        at java.base/java.lang.ProcessBuilder.start(ProcessBuilder.java:1110)

@vpa1977 vpa1977 marked this pull request as ready for review March 1, 2024 03:13
@openjdk openjdk bot added the rfr Pull request is ready for review label Mar 1, 2024
@mlbridge
Copy link

mlbridge bot commented Mar 1, 2024

Webrevs

@AlanBateman
Copy link
Contributor

Would it be possible to expand a bit on what is going on here? Are you saying that in the course of upgrading a JDK that you somehow get into a state where jspawnhelper has been replaced with a version from a newer JDK? Is the real issue here that the upgrade didn't shutdown the application and/or something copied over an existing installation during the upgrade?

@vpa1977 vpa1977 closed this Mar 1, 2024
@vpa1977
Copy link
Contributor Author

vpa1977 commented Mar 4, 2024

Would it be possible to expand a bit on what is going on here? Are you saying that in the course of upgrading a JDK that you somehow get into a state where jspawnhelper has been replaced with a version from a newer JDK? Is the real issue here that the upgrade didn't shutdown the application and/or something copied over an existing installation during the upgrade?

It appears that answer is yes (in Debian and Ubuntu) if the machine is running unattended-upgrades service and openjdk is not blacklisted from upgrades.
I believe we should blacklist openjdk packages by default to avoid this situation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core-libs core-libs-dev@openjdk.org rfr Pull request is ready for review
Development

Successfully merging this pull request may close these issues.

2 participants