OpenJDK / amber / amber
changeset 56622:57b93b113ec0
8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error
Reviewed-by: rriggs, martin, fweimer
author | stuefe |
---|---|
date | Wed, 05 Jun 2019 09:12:45 +0200 |
parents | 74832e7b5cad |
children | 7e2238451585 |
files | src/java.base/unix/native/libjava/ProcessImpl_md.c src/java.base/unix/native/libjava/childproc.c src/java.base/unix/native/libjava/childproc.h |
diffstat | 3 files changed, 51 insertions(+), 0 deletions(-) [+] |
line wrap: on
line diff
--- a/src/java.base/unix/native/libjava/ProcessImpl_md.c Wed Jun 05 14:07:14 2019 -0400 +++ b/src/java.base/unix/native/libjava/ProcessImpl_md.c Wed Jun 05 09:12:45 2019 +0200 @@ -655,6 +655,18 @@ c->redirectErrorStream = redirectErrorStream; c->mode = mode; + /* In posix_spawn mode, require the child process to signal aliveness + * right after it comes up. This is because there are implementations of + * posix_spawn() which do not report failed exec()s back to the caller + * (e.g. glibc, see JDK-8223777). In those cases, the fork() will have + * worked and successfully started the child process, but the exec() will + * have failed. There is no way for us to distinguish this from a target + * binary just exiting right after start. + * + * Note that we could do this additional handshake in all modes but for + * prudence only do it when it is needed (in posix_spawn mode). */ + c->sendAlivePing = (mode == MODE_POSIX_SPAWN) ? 1 : 0; + resultPid = startChild(env, process, c, phelperpath); assert(resultPid != 0); @@ -674,6 +686,30 @@ } close(fail[1]); fail[1] = -1; /* See: WhyCantJohnnyExec (childproc.c) */ + /* If we expect the child to ping aliveness, wait for it. */ + if (c->sendAlivePing) { + switch(readFully(fail[0], &errnum, sizeof(errnum))) { + case 0: /* First exec failed; */ + waitpid(resultPid, NULL, 0); + throwIOException(env, 0, "Failed to exec spawn helper."); + goto Catch; + case sizeof(errnum): + assert(errnum == CHILD_IS_ALIVE); + if (errnum != CHILD_IS_ALIVE) { + /* Should never happen since the first thing the spawn + * helper should do is to send an alive ping to the parent, + * before doing any subsequent work. */ + throwIOException(env, 0, "Bad code from spawn helper " + "(Failed to exec spawn helper."); + goto Catch; + } + break; + default: + throwIOException(env, errno, "Read failed"); + goto Catch; + } + } + switch (readFully(fail[0], &errnum, sizeof(errnum))) { case 0: break; /* Exec succeeded */ case sizeof(errnum):
--- a/src/java.base/unix/native/libjava/childproc.c Wed Jun 05 14:07:14 2019 -0400 +++ b/src/java.base/unix/native/libjava/childproc.c Wed Jun 05 09:12:45 2019 +0200 @@ -315,6 +315,13 @@ const ChildStuff* p = (const ChildStuff*) arg; int fail_pipe_fd = p->fail[1]; + if (p->sendAlivePing) { + /* Child shall signal aliveness to parent at the very first + * moment. */ + int code = CHILD_IS_ALIVE; + restartableWrite(fail_pipe_fd, &code, sizeof(code)); + } + /* Close the parent sides of the pipes. Closing pipe fds here is redundant, since closeDescriptors() would do it anyways, but a little paranoia is a good thing. */
--- a/src/java.base/unix/native/libjava/childproc.h Wed Jun 05 14:07:14 2019 -0400 +++ b/src/java.base/unix/native/libjava/childproc.h Wed Jun 05 09:12:45 2019 +0200 @@ -101,6 +101,7 @@ const char **envv; const char *pdir; int redirectErrorStream; + int sendAlivePing; } ChildStuff; /* following used in addition when mode is SPAWN */ @@ -114,6 +115,13 @@ int parentPathvBytes; /* total number of bytes in parentPathv array */ } SpawnInfo; +/* If ChildStuff.sendAlivePing is true, child shall signal aliveness to + * the parent the moment it gains consciousness, before any subsequent + * pre-exec errors could happen. + * This code must fit into an int and not be a valid errno value on any of + * our platforms. */ +#define CHILD_IS_ALIVE 65535 + /** * The cached and split version of the JDK's effective PATH. * (We don't support putenv("PATH=...") in native code)