Patchwork [2,of,3] rust-chg: remove SIGCHLD handler which won't work in oxidized chg

login
register
mail settings
Submitter Yuya Nishihara
Date Oct. 7, 2018, 2:45 p.m.
Message ID <e707e0d5ed0fc1b99b33.1538923529@mimosa>
Download mbox | patch
Permalink /patch/35541/
State New
Headers show

Comments

Yuya Nishihara - Oct. 7, 2018, 2:45 p.m.
# 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.
Augie Fackler - Oct. 10, 2018, 8:24 a.m.
On Sun, Oct 07, 2018 at 11:45:29PM +0900, Yuya Nishihara wrote:
> # 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

queued, thanks

Patch

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))
     }