On Wed, 17 May 2023 12:33:48 GMT, Volker Simonis <simo...@openjdk.org> wrote:

>> Since JDK13, executing commands in a sub-process defaults to the so called 
>> `POSIX_SPAWN` launching mechanism (i.e. 
>> `-Djdk.lang.Process.launchMechanism=POSIX_SPAWN`) on Linux. This works by 
>> using `posix_spawn(3)` to firstly start a tiny helper program called 
>> `jspawnhelper` in a subprocess. In a second step, `jspawnhelper` reads the 
>> command data from the parent Java process over a Unix pipe and finally 
>> executes (i.e. `execvp(3)`) the requested command.
>> 
>> In cases where the parent process terminates abnormally before 
>> `jspawnhelper` has read all the expected data from the pipe, `jspawnhelper` 
>> will block indefinitely on the reading end of the pipe. This is especially 
>> harmful if the parent process had open sockets, because in that case, the 
>> forked `jspawnhelper` process will inherit them and keep all the 
>> corresponding ports open effectively preventing other processes to bind to 
>> them. Notice that this is not an abstract scenario. We've observed this 
>> regularly in production with services which couldn't be restarted after a 
>> crash after migrating to JDK 17.
>> 
>> The fix of the issue is rather trivial. `jspawnhelper` has to close its 
>> writing end of the pipe which connects it with the parent Java process 
>> *before* starting to read from that pipe such that reads from the pipe can 
>> immediately return with EOF if the parent process terminates abnormally.
>> 
>> Also did some cleanup:
>>  - in `jspawnhelper.c` correctly chek the result of `readFully()`
>>  - in `ProcessImpl_md.c` use `restartableWrite()` instead of `write()` to 
>> write the command data from the parent process to `jspawnhelper`
>>  - in `childproc.{c,h}` remove remaining traces of `MODE_CLONE` because 
>> that's long gone.
>
> Volker Simonis has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Added test case

src/java.base/unix/native/libjava/childproc.c line 408:

> 406:  * or the newly created jspawnhelper child process during the execution 
> of
> 407:  * Java_java_lang_ProcessImpl_forkAndExec().
> 408:  * See: test/jdk/java/lang/ProcessBuilder/JspawnhelperProtocol.java

Is there a file missing from the commit? Where is JspawnhelperProtocol.java?
It might be useful to include the bug number; though generally they can get 
stale and uninteresting quickly.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/13956#discussion_r1196651832

Reply via email to