8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error
Reviewed-by: rriggs, martin, fweimer
--- 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)