X-Git-Url: https://git.sesse.net/?p=cubemap;a=blobdiff_plain;f=httpinput.cpp;h=62ed663611402cef5d4517cef00d8033300391bc;hp=274cc2687b160d45e365fc7f4daafb665b9d2660;hb=0229f4414b3b63934c057030deb88cbf6926bb1f;hpb=e3f2936e3c9ff3b5569759c1aaed16f03bf728f8

diff --git a/httpinput.cpp b/httpinput.cpp
index 274cc26..62ed663 100644
--- a/httpinput.cpp
+++ b/httpinput.cpp
@@ -75,11 +75,12 @@ HTTPInput::HTTPInput(const InputProto &serialized)
 	  http_header(serialized.http_header()),
 	  stream_header(serialized.stream_header()),
 	  has_metacube_header(serialized.has_metacube_header()),
-	  sock(serialized.sock())
+	  sock(serialized.sock()),
+	  child_pid(serialized.child_pid())
 {
 	// Set back the close-on-exec flag for the socket.
 	// (This can't leak into a child, since we haven't been started yet.)
-	fcntl(sock, F_SETFD, 1);
+	fcntl(sock, F_SETFD, O_CLOEXEC);
 
 	pending_data.resize(serialized.pending_data().size());
 	memcpy(&pending_data[0], serialized.pending_data().data(), serialized.pending_data().size());
@@ -112,6 +113,13 @@ void HTTPInput::close_socket()
 		safe_close(sock);
 		sock = -1;
 	}
+	if (child_pid != -1) {
+		// Kill the child process group, forcibly.
+		// TODO: Consider using a pidfd on newer kernels, so that we're guaranteed
+		// never to kill the wrong process.
+		kill(-child_pid, SIGKILL);
+	}
+	child_pid = -1;
 
 	lock_guard<mutex> lock(stats_mutex);
 	stats.connect_time = -1;
@@ -134,6 +142,7 @@ InputProto HTTPInput::serialize() const
 	serialized.set_pending_data(string(pending_data.begin(), pending_data.end()));
 	serialized.set_has_metacube_header(has_metacube_header);
 	serialized.set_sock(sock);
+	serialized.set_child_pid(child_pid);
 	serialized.set_bytes_received(stats.bytes_received);
 	serialized.set_data_bytes_received(stats.data_bytes_received);
 	if (isfinite(stats.latency_sec)) {
@@ -247,23 +256,33 @@ int HTTPInput::open_child_process(const string &cmdline)
 	posix_spawn_file_actions_adddup2(&actions, devnullfd, 0);
 	posix_spawn_file_actions_adddup2(&actions, pipefd[1], 1);
 
-	pid_t child_pid;
+	// Make the process a leader of its own process group, so that we can easily
+	// kill it and any of its child processes (unless it's started new process
+	// groups itself, of course).
+	posix_spawnattr_t attr;
+	posix_spawnattr_init(&attr);
+	posix_spawnattr_setflags(&attr, POSIX_SPAWN_SETPGROUP);
+	posix_spawnattr_setpgroup(&attr, 0);
+
 	char * const argv[] = {
 		strdup("/bin/sh"),
 		strdup("-c"),
 		strdup(path.c_str()),
 		nullptr
 	};
-	int err = posix_spawn(&child_pid, "/bin/sh", &actions, /*attrp=*/nullptr, argv, /*envp=*/nullptr);
+	int err = posix_spawn(&child_pid, "/bin/sh", &actions, &attr, argv, /*envp=*/nullptr);
 	posix_spawn_file_actions_destroy(&actions);
+	posix_spawnattr_destroy(&attr);
 	free(argv[0]);
 	free(argv[1]);
 	free(argv[2]);
+	close(devnullfd);
 	close(pipefd[1]);
 
 	if (err == 0) {
 		return pipefd[0];
 	} else {
+		child_pid = -1;
 		log_perror(cmdline.c_str());
 		close(pipefd[0]);
 		return -1;
@@ -383,8 +402,21 @@ void HTTPInput::do_work()
 		switch (state) {
 		case NOT_CONNECTED: {
 			// Reap any exited children.
-			int wstatus;
-			while (waitpid(-1, &wstatus, WNOHANG) != -1 || errno == EINTR) ;
+			int wstatus, err;
+			do {
+				err = waitpid(-1, &wstatus, WNOHANG);
+				if (err == -1) {
+					if (errno == EINTR) {
+						continue;
+					}
+					if (errno == ECHILD) {
+						break;
+					}
+					log_perror("waitpid");
+					break;
+				}
+			} while (err != 0);
+			child_pid = -1;
 
 			request.clear();
 			request_bytes_sent = 0;