On Mon, 9 Mar 2026 03:25:24 GMT, Roger Riggs <[email protected]> wrote:

>> Thomas Stuefe has updated the pull request with a new target base due to a 
>> merge or a rebase. The incremental webrev excludes the unrelated changes 
>> brought in by the merge/rebase. The pull request contains nine additional 
>> commits since the last revision:
>> 
>>  - remove accidental whitespace
>>  - prevent accidental overwrite of childenv pipe
>>  - Roger Feedback
>>  - Feedback Volker
>>  - Fix builds for glibc < 2.34
>>  - wip
>>  - copyrights
>>  - new test
>>  - start
>
> Related to Ubuntuy 25-10 and ProcessBuilder/Basic.java tests labeled "PATH 
> search algorithm".
> 
> Ubuntu 25-10 has changed /usr/bin/true and /usr/bin/false to be links to a 
> "multi-call binary" that implements many command shell commands.
> A subset of the ProcessBuilder/Basic.java tests labeled "PATH search 
> algorithm" fail as a result.
> The tests make copies of /usr/bin/true and /usr/bin/false into various 
> directories using the name "prog".
> The PATH is set to combinations of the direcrtories.
> `prog` is invoked and the exit status should match the expected statue of 
> /usr/bin/true or /usr/bin/false.
> However, when the multi-call binary is invoked as `prog` the exit status does 
> not match.
> The tests should also be failing on Ubuntu 15-10 in the mainline and more 
> recent JDK releases.
> There should be a separate issue for that.

Hi @RogerRiggs 

I added two changes to the patch.

----------

The first one, 
https://github.com/openjdk/jdk/pull/29939/changes/0b97298a38e084e11690024be02b56e55858466e,
 deals with the case I described above in 
https://github.com/openjdk/jdk/pull/29939#issuecomment-4012489335:

1) I first move stdin/out/err to their respective 0,1,2 places. From here on we 
don't care anymore if someone overwrites the original stdin/out/err.

2) then I calculate a temporary file descriptor that must not be the 
child_fail, must not be too low, and must not run into the OPEN_MAX limit.

3) then I:
  3.1) first dup2 childenv to temp fd. The temp fd will stay valid even if the 
subsequent step happens to overwrite the original childenv fd.
  3.2) dup2 the fail pipe fd to its final place (3).
  3.3) dup2 temp fd to its place (4)

All file descriptors above 4 are closed as usually at the final exec.

----

The second patch is this: 
https://github.com/openjdk/jdk/pull/29939/changes/5cbac2b8cf91869a96321d546a6c6d64cdff8c16

I changed the MacOS workaround for the "CLOEXEC file descriptors are closed by 
the kernel before posix_spawn processes the dup2 actions" problem. 

The original workaround did another dup2 copy. But that was a bit stupid, since 
these copies, when leaking to other processes, would also keep the pipes open, 
thereby negating the whole point of the patch (and my new manual 
ConcNativeForkTest showed that MacOS for the posix_spawn mode, but I missed it).

The new workaround uses the MacOS-specific 
`posix_spawn_file_actions_addinherit_np` to tell the kernel "Ignore the CLOEXEC 
flag on these file descriptors". This seems to work on my machine. Not sure why 
it did not work beforehand.

----

I reran tier1 jdk tests successfully on
- Linux x64 with glibc
- Linux x64 with muslc
- MacOS arm64
- AIX

I also ran the manual reproducer ConcNativeForkTest on all of these platforms, 
they all succeeded.

-------------

PR Comment: https://git.openjdk.org/jdk/pull/29939#issuecomment-4023300965

Reply via email to