From patchwork Sun Oct  7 14:45:29 2018
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: [2, of,
 3] rust-chg: remove SIGCHLD handler which won't work in oxidized chg
From: Yuya Nishihara <yuya@tcha.org>
X-Patchwork-Id: 35541
Message-Id: <e707e0d5ed0fc1b99b33.1538923529@mimosa>
To: mercurial-devel@mercurial-scm.org
Date: Sun, 07 Oct 2018 23:45:29 +0900

# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 1537795189 -32400
#      Mon Sep 24 22:19:49 2018 +0900
# Node ID e707e0d5ed0fc1b99b33004067f5fd568d2c15c8
# Parent  60d4ea6eefdc897ccd8f5a9553a7a1ad39de8a7b
rust-chg: remove SIGCHLD handler which won't work in oxidized chg

Since pager is managed by the Rust part, the C code doesn't know the pager
pid. I could make the Rust part teach the pid to C, but still installing
SIGCHLD handler seems horrible idea since we no longer use handcrafted
low-level process management functions.

Instead, I'm thinking of adding async handler to send SIGPIPE at the exit
of the pager.

diff --git a/rust/chg/src/sighandlers.c b/rust/chg/src/sighandlers.c
--- a/rust/chg/src/sighandlers.c
+++ b/rust/chg/src/sighandlers.c
@@ -11,16 +11,8 @@
 #include <errno.h>
 #include <signal.h>
 #include <string.h>
-#include <sys/wait.h>
 #include <unistd.h>
 
-#ifdef __GNUC__
-#define UNUSED_ __attribute__((unused))
-#else
-#define UNUSED_
-#endif
-
-static pid_t pagerpid = 0;
 static pid_t peerpgid = 0;
 static pid_t peerpid = 0;
 
@@ -65,17 +57,6 @@ static void handlestopsignal(int sig)
 		return;
 }
 
-static void handlechildsignal(int sig UNUSED_)
-{
-	if (peerpid == 0 || pagerpid == 0)
-		return;
-	/* if pager exits, notify the server with SIGPIPE immediately.
-	 * otherwise the server won't get SIGPIPE if it does not write
-	 * anything. (issue5278) */
-	if (waitpid(pagerpid, NULL, WNOHANG) == pagerpid)
-		kill(peerpid, SIGPIPE);
-}
-
 /*
  * Installs signal handlers.
  *
@@ -131,11 +112,6 @@ int setupsignalhandler(pid_t pid, pid_t 
 	sa.sa_flags = SA_RESTART;
 	if (sigaction(SIGTSTP, &sa, NULL) < 0)
 		return -1;
-	/* get notified when pager exits */
-	sa.sa_handler = handlechildsignal;
-	sa.sa_flags = SA_RESTART;
-	if (sigaction(SIGCHLD, &sa, NULL) < 0)
-		return -1;
 
 	return 0;
 }
@@ -164,8 +140,6 @@ int restoresignalhandler(void)
 		return -1;
 	if (sigaction(SIGTSTP, &sa, NULL) < 0)
 		return -1;
-	if (sigaction(SIGCHLD, &sa, NULL) < 0)
-		return -1;
 
 	/* ignore Ctrl+C while shutting down to make pager exits cleanly */
 	sa.sa_handler = SIG_IGN;
diff --git a/rust/chg/src/uihandler.rs b/rust/chg/src/uihandler.rs
--- a/rust/chg/src/uihandler.rs
+++ b/rust/chg/src/uihandler.rs
@@ -53,6 +53,10 @@ impl SystemHandler for ChgUiHandler {
             .spawn_async()?;
         let pin = pager.stdin().take().unwrap();
         procutil::set_blocking_fd(pin.as_raw_fd())?;
+        // TODO: if pager exits, notify the server with SIGPIPE immediately.
+        // otherwise the server won't get SIGPIPE if it does not write
+        // anything. (issue5278)
+        // kill(peerpid, SIGPIPE);
         tokio::spawn(pager.map(|_| ()).map_err(|_| ()));  // just ignore errors
         Ok((self, pin))
     }