From ced20ad0d962a8809f53cbd214313cad13ad5fab Mon Sep 17 00:00:00 2001
From: Oswald Buddenhagen <ossi@users.sf.net>
Date: Sun, 3 Jul 2022 11:52:20 +0200
Subject: [PATCH] fix Tunnel leaving behind zombies

this generally went unnoticed, as the tunnel usually terminates right
before we exit anyway. however, if multiple Channels are synced, it may
become visible.

this is a "shotgun" implementation, where the main loop just reaps all
unclaimed children.
arguably, it would be cleaner if each socket actually tracked its own
process, but getting synchronous kills+waits right is tricky, so we
continue to pretend that there is no process as far as the socket layer
is concerned.

poll()/select() are not restartable, so they need EINTR handling now
that SIGCHLD is actually delivered.
---
 src/main.c      | 40 ++++++++++++++++++++++++++++++++++++++++
 src/main_list.c |  2 +-
 src/main_p.h    |  1 +
 src/main_sync.c |  2 +-
 src/util.c      |  4 ++++
 5 files changed, 47 insertions(+), 2 deletions(-)

diff --git a/src/main.c b/src/main.c
index 993a176..8581b0c 100644
--- a/src/main.c
+++ b/src/main.c
@@ -89,6 +89,26 @@ PACKAGE " " VERSION " - mailbox synchronizer\n"
 	exit( code );
 }
 
+static int child_pipe[2];
+static notifier_t child_notifier;
+
+static void
+childHandler( int n ATTR_UNUSED )
+{
+	// We can't just reap everything here, as we might steal children
+	// from popen(). Let the main loop handle it synchronously instead.
+	char dummy = 0;
+	write( child_pipe[1], &dummy, 1 );
+}
+
+static void
+childReaper( int events ATTR_UNUSED, void *aux ATTR_UNUSED )
+{
+	char dummy;
+	while (read( child_pipe[0], &dummy, 1 ) == 1) {}
+	while (waitpid( -1, NULL, WNOHANG ) > 0) {}
+}
+
 #ifdef __linux__
 static void ATTR_NORETURN
 crashHandler( int n )
@@ -543,9 +563,29 @@ main( int argc, char **argv )
 
 	signal( SIGPIPE, SIG_IGN );
 
+	if (pipe( child_pipe )) {
+		perror( "pipe" );
+		return 1;
+	}
+	fcntl( child_pipe[0], F_SETFL, O_NONBLOCK );
+	fcntl( child_pipe[1], F_SETFL, O_NONBLOCK );
+	init_notifier( &child_notifier, child_pipe[0], childReaper, NULL );
+	conf_notifier( &child_notifier, 0, POLLIN );
+	struct sigaction sa = { 0 };
+	sa.sa_handler = childHandler;
+	sa.sa_flags = SA_NOCLDSTOP | SA_RESTART;
+	sigaction( SIGCHLD, &sa, NULL );
+
 	if (mvars->list_stores)
 		list_stores( mvars, argv + oind );
 	else
 		sync_chans( mvars, argv + oind );
 	return mvars->ret;
 }
+
+void
+cleanup_mainloop( void )
+{
+	cleanup_drivers();
+	wipe_notifier( &child_notifier );
+}
diff --git a/src/main_list.c b/src/main_list.c
index 48bf4dd..c4aa63e 100644
--- a/src/main_list.c
+++ b/src/main_list.c
@@ -126,7 +126,7 @@ do_list_stores( list_vars_t *lvars )
 	  next:
 		advance_store( lvars );
 	}
-	cleanup_drivers();
+	cleanup_mainloop();
 }
 
 static void
diff --git a/src/main_p.h b/src/main_p.h
index 75f619c..6aa5fe2 100644
--- a/src/main_p.h
+++ b/src/main_p.h
@@ -21,5 +21,6 @@ typedef struct {
 
 void sync_chans( core_vars_t *cvars, char **argv );
 void list_stores( core_vars_t *cvars, char **argv );
+void cleanup_mainloop( void );
 
 #endif
diff --git a/src/main_sync.c b/src/main_sync.c
index 24a88c9..226e324 100644
--- a/src/main_sync.c
+++ b/src/main_sync.c
@@ -496,7 +496,7 @@ do_sync_chans( main_vars_t *mvars )
 	  next:
 		advance_chan( mvars );
 	}
-	cleanup_drivers();
+	cleanup_mainloop();
 	if (!mvars->cvars->list && (DFlags & PROGRESS))
 		wipe_wakeup( &stats_wakeup );
 }
diff --git a/src/util.c b/src/util.c
index 3ba7f91..e697cfb 100644
--- a/src/util.c
+++ b/src/util.c
@@ -1110,6 +1110,8 @@ event_wait( void )
 	case 0:
 		return;
 	case -1:
+		if (errno == EINTR)
+			return;
 		perror( "poll() failed in event loop" );
 		abort();
 	default:
@@ -1162,6 +1164,8 @@ event_wait( void )
 	case 0:
 		return;
 	case -1:
+		if (errno == EINTR)
+			return;
 		perror( "select() failed in event loop" );
 		abort();
 	default: