JDK-8325567 : jspawnhelper without args fails with segfault
  • Type: Bug
  • Component: core-libs
  • Sub-Component: java.lang
  • Affected Version: 11,17,21,22
  • Priority: P4
  • Status: Closed
  • Resolution: Fixed
  • OS: linux,os_x
  • Submitted: 2024-02-09
  • Updated: 2024-07-22
  • Resolved: 2024-03-08
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 17 JDK 21 JDK 22 JDK 23
17.0.13Fixed 21.0.4Fixed 22.0.2Fixed 23 b14Fixed
Related Reports
Relates :  
Relates :  
Description
When `jspawnhelper` is run without args it outputs:

"This command is not for general use and should only be run as the result of a call to ProcessBuilder.start() or Runtime.exec() in a java application"

JDK-8310265 broke this. Now `jspawnhelper` fails with sigfault because of the change:

-    /* argv[0] contains the fd number to read all the child info */
+   /* argv[1] contains the fd number to read all the child info */
    int r, fdin, fdout;

-    r = sscanf (argv[argc-1], "%d:%d", &fdin, &fdout);
+   r = sscanf (argv[1], "%d:%d", &fdin, &fdout);

Without args, argc is 1, so argv[1] is an out of bound access.
Comments
[jdk17u-fix-request] Approval Request from Elif Aslan Clean backport to address the segmentation fault issue that occurs when jspawnhelper is called without arguments. There is a new test added to verify the behavior in such cases.New regression test fails without the fix, and passes with the change.
06-06-2024

Maybe this is an issue for skara? It could check whether the change is backported to earlier releases, and not label "ready" if it is not. There could be a command \ignore_earlier_releases or the like for cases where this is not applicable.
10-04-2024

Thanks for highlighting these issues [~goetz] - I'm following up and will reply back shortly.
10-04-2024

Hi Sean, Some more examples where also Oracle does not follow this rule: JDK-8324933, JDK-8326106. And even for LTS releases: JDK-8318322, JDK-8327631.
10-04-2024

Thanks for sharing your thoughts. I've done my fair share of backports also and I don't see porting to the newest Update release as a major cost. The patch applies 99.9% of the time (given how little the code would have diverged from the main line in a few months)., Having an engineer decide if the patch should go into the latest JDK Update release is probably just as tricky a task for some. At Oracle, my understanding is that a backported fix always goes into the latest JDK Update release before porting to older JDK family releases. I appreciate that this process gets blurry as we approach rampdown of the X.0.2 (non-LTS) update releases since there's little merit to integrating fixes late in the release cycle if they won't be released in the actual product. My main concern is the image of OpenJDK product quality here. Having users on the latest update release enjoying the latest JDK language features shouldn't come with the baggage of them wondering whether the product may be defective compared to older JDK family releases.
05-04-2024

I agree with both of you, fixes should be backported to the youngest supported release first, but there is a point where it's no more worth while if a release is coming to it's end. Actually fixes reaching 22.0.2 are only 2-3 months productive. But this one might be a nice-to-have.
04-04-2024

The backports always come with a cost/benefit choice. The backporting cost is not as trivial as issuing /backport command. The benefit is often taken down by user impact and release lifecycle. We often make a deliberate choice not to backport things that do not have a favorable cost/benefit ratio, and often that choice is JDK-version specific. Of course, it is preferable to maintain the inclusivity for fixes, so that all JDKs >= $X have the fix, but it is a preference, not the rule. Sometimes, skipping backports to STS-es is a practical thing to do. I personally appreciate this practice after doing hundreds of backports across many releases :) Take this concrete patch as the example: it is not that clear the time spent triaging, backporting, testing, following up on problems for a patch that does not have a direct user impact for a release that would be EOL in 5 months is a time well spent. For 21u and 17u the cost/benefit calculation is offset by the eventual backport of JDK-8325621 made clean by this one.
04-04-2024

Thanks for the feedback [~shade] - I'm surprised to see this practice happening to be honest. Users of OpenJDK 22u should enjoy at least the same level of quality as that on the earlier OpenJDK 21u releases. Yes - there is some extra overhead to integration for 22u. The Skara infrastructure has made this trivial though via the /backport command functionality.
04-04-2024

A pull request was submitted for review. URL: https://git.openjdk.org/jdk22u/pull/126 Date: 2024-04-04 09:02:11 +0000
04-04-2024

[jdk22u-fix-request] Approval Request from Aleksey Shipilëv Clean backport to improve jspawnhelper resiliency and provide the grounds for later backports. Relevant tests pass, including the new test. Risk is low: touches the production code paths, but in simple manner.
04-04-2024

For some patches, it just adds to churn to backport to STS releases that would EOL very soon anyway. I also personally see there is often an implicit division of labor: backoports to 22u and other STSes are normally done by Oracle folks, while we are doing the LTS backports that Oracle folks do not normally do. We can do a jdk22u backport for this one too, of course.
04-04-2024

[~easlan] [~goetz] - is there a reason why these fixes aren't ported to jdk22u project first ? I thought that was best practice.
04-04-2024

[jdk21u-fix-request] Approval Request from Elif Aslan Clean backport to address the segmentation fault issue that occurs when jspawnhelper is called without arguments,.
03-04-2024

A pull request was submitted for review. URL: https://git.openjdk.org/jdk21u-dev/pull/414 Date: 2024-03-27 15:54:06 +0000
27-03-2024

A pull request was submitted for review. URL: https://git.openjdk.org/jdk17u-dev/pull/2339 Date: 2024-03-27 15:51:18 +0000
27-03-2024

Changeset: 26274709 Author: Elif Aslan <elifasln@amazon.com> Committer: Evgeny Astigeevich <eastigeevich@openjdk.org> Date: 2024-03-08 22:09:18 +0000 URL: https://git.openjdk.org/jdk/commit/262747094670b00ac63463a059074afa9b81d8a4
08-03-2024

A pull request was submitted for review. URL: https://git.openjdk.org/jdk/pull/18112 Date: 2024-03-04 21:19:26 +0000
04-03-2024

Thanks! This Ubuntu observation/bug actually contributes a lot for understanding some of the customer failures we have seen with jspawnhelper crashes -- thanks for linking it here. And it gives JDK-8325621 even more importance, even if it is nominally not a JDK bug to run into jspawnhelper incompatibilities due to issues with distro upgrade processes. Please track/work JDK-8325621 and see what makes sense to add there from your PR, when the PR for JDK-8325621 appears.
01-03-2024

I apologise for making the PR, I have closed it, and I agree that the root cause is probably best resolved in unattended upgrades. Regarding the crash itself, probably its still worth adding a check for argc and shutdown cleanly.
01-03-2024

I agree with David here. Upgrading the JDK like this is not safe. The fact that jspawnhelper falls into this trap is the _easily observable_ failure mode, but there would be many more. This is an unattended upgrades problem, the unattended upgrades should either never upgrade openjdk packages, or suggest restarting the Java services if openjdk package is upgraded. Process-wise, I believe jspawnhelper compatibility checks should be covered by JDK-8325621, not this bug. This bug is specifically about not crashing with SEGV. Also, it is a considerably bad contribution style to suggest a PR for the bug already assigned to somebody else, at least for this reason: assignee usually understands much more context, and probably invested considerable time in it. If you wanted to do this work, you needed to ask first to hand over the bug assignment. I suggest Vladimir to close his PR and work it out the rest with Chad in JDK-8325621.
01-03-2024

> Unattended upgrades will replace the Java machine, but the active process will still use the old protocol. That seems like extremely bad practice! If you replace the JDK/JRE on disk while there is still a running Java process then it could try and load incompatible libraries from the new on-disk image. That could result in all manner of errors!
01-03-2024

A pull request was submitted for review. URL: https://git.openjdk.org/jdk/pull/18074 Date: 2024-03-01 01:50:46 +0000
01-03-2024

This is a real problem for the long running Java processes, such as Jenkins workers. Unattended upgrades will replace the Java machine, but the active process will still use the old protocol. jspawnhelper should probably fall back on old behaviour in this case. See [1] for investigation by Dimitry Andric. [1] https://bugs.launchpad.net/ubuntu/+source/openjdk-17/+bug/2055280
29-02-2024

Please set the fix-version to "TBD" until the fix is pushed. tnx
26-02-2024

I read the adjacent code, and cannot see how JDK could get into this condition. The only invocation of jspawnhelper is here, and it always comes with proper arguments: https://github.com/openjdk/jdk/blob/1358850aa63a2874031ca33eba278432fd09d6ab/src/java.base/unix/native/libjava/ProcessImpl_md.c#L497-L504 So this would only affect whoever is invoking jspawnhelper directly. But that would also run into problems when jspawnhelper protocol changes like in JDK-8310265. Whoever uses jspawnhelper directly is in the world of (self-inflicted) pain at that point. I believe we should still plug this corner case to at least keep printing the warning message correctly.
12-02-2024