[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

[Nbd] [PATCH 11/11] nbd-server: fix signal handling



Reset SIGCHLD and SIGTERM handlers to SIG_DFL in child processes because
it is not their business to handle these signals.  Block SIGCHLD and
SIGTERM for the short time of fork, changing signal handlers and
changing "children" hash table to avoid race conditions with nasty
consequences.  Fix SIGTERM handler to always unlink pidfile.

Signed-off-by: Dmitry V. Levin <ldv@...1147...>
---
 nbd-server.c |   23 ++++++++++++++---------
 1 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/nbd-server.c b/nbd-server.c
index 80681c7..fc5ae2d 100644
--- a/nbd-server.c
+++ b/nbd-server.c
@@ -1087,10 +1087,8 @@ void sigchld_handler(int s) {
  **/
 void killchild(gpointer key, gpointer value, gpointer user_data) {
 	pid_t *pid=value;
-	int *parent=user_data;
 
 	kill(*pid, SIGTERM);
-	*parent=1;
 }
 
 /**
@@ -1099,13 +1097,8 @@ void killchild(gpointer key, gpointer value, gpointer user_data) {
  * is severely wrong).
  **/
 void sigterm_handler(int s) {
-	int parent=0;
-
-	g_hash_table_foreach(children, killchild, &parent);
-
-	if(parent) {
-		unlink(pidfname);
-	}
+	g_hash_table_foreach(children, killchild, NULL);
+	unlink(pidfname);
 
 	exit(EXIT_SUCCESS);
 }
@@ -2198,9 +2191,16 @@ handle_connection(GArray *servers, int net, SERVER *serve, CLIENT *client)
 	if (!dontfork) {
 		pid_t pid;
 		int i;
+		sigset_t newset;
+		sigset_t oldset;
 
+		sigemptyset(&newset);
+		sigaddset(&newset, SIGCHLD);
+		sigaddset(&newset, SIGTERM);
+		sigprocmask(SIG_BLOCK, &newset, &oldset);
 		if ((pid = fork()) < 0) {
 			msg3(LOG_INFO,"Could not fork (%s)",strerror(errno)) ;
+			sigprocmask(SIG_SETMASK, &oldset, NULL);
 			goto handle_connection_out;
 		}
 		if (pid > 0) { /* parent */
@@ -2209,9 +2209,14 @@ handle_connection(GArray *servers, int net, SERVER *serve, CLIENT *client)
 			pidp = g_malloc(sizeof(pid_t));
 			*pidp = pid;
 			g_hash_table_insert(children, pidp, pidp);
+			sigprocmask(SIG_SETMASK, &oldset, NULL);
 			goto handle_connection_out;
 		}
 		/* child */
+		signal(SIGCHLD, SIG_DFL);
+		signal(SIGTERM, SIG_DFL);
+		sigprocmask(SIG_SETMASK, &oldset, NULL);
+
 		g_hash_table_destroy(children);
 		children = NULL;
 		for(i=0;i<servers->len;i++) {
-- 
ldv



Reply to: