changeset 28880:5063f721de6d

We really shouldn't be doing a whole lot in our signal handler. The signal handler can get called in the middle of a malloc, for example, and if the signal handler tries to allocate memory then the program can deadlock. This change works around that problem by having the signal handler write to a pipe. Our main program watches the pipe for incoming data and performs the appropriate action. This patch is from Shaun Lindsay at Meebo. Fixes #10324

--- a/pidgin/gtkmain.c	Wed Nov 04 22:16:51 2009 +0000
+++ b/pidgin/gtkmain.c	Wed Nov 04 22:47:18 2009 +0000
@@ -143,6 +143,10 @@
 }
 
 #ifdef HAVE_SIGNAL_H
+static char *segfault_message;
+
+static int signal_sockets[2];
+
 static void sighandler(int sig);
 
 /*
@@ -168,31 +172,60 @@
 	}
 }
 
-char *segfault_message;
+static void sighandler(int sig)
+{
+	ssize_t written;
+
+	/*
+	 * We won't do any of the heavy lifting for the signal handling here
+	 * because we have no idea what was interrupted.  Previously this signal
+	 * handler could result in some calls to malloc/free, which can cause
+	 * deadlock in libc when the signal handler was interrupting a previous
+	 * malloc or free.  So instead we'll do an ugly hack where we write the
+	 * signal number to one end of a socket pair.  The other half of the
+	 * socket pair is watched by our main loop.  When the main loop sees new
+	 * data on the socket it reads in the signal and performs the appropriate
+	 * action without fear of interrupting stuff.
+	 */
+	if (sig == SIGSEGV) {
+		fprintf(stderr, "%s", segfault_message);
+		abort();
+		return;
+	}
 
-/*
- * This signal handler shouldn't be touching this much stuff.
- * It should just set a flag and return, and something else in
- * Pidgin should monitor the flag to see if something needs to
- * be done.  Because the signal handler interrupts the program,
- * it could be called in the middle of adding a new connection
- * to the list of connections, and then if we try to disconnect
- * all connections it could lead to a crash because the linked
- * list of connections could be in a weird state.  But, well,
- * this signal handler probably isn't called very often, so it's
- * not a big deal.
- */
-static void
-sighandler(int sig)
+	written = write(signal_sockets[0], &sig, sizeof(int));
+	if (written < 0 || written != sizeof(int)) {
+		/* This should never happen */
+		purple_debug_error("sighandler", "Received signal %d but only "
+				"wrote %" G_GSSIZE_FORMAT " bytes out of %"
+				G_GSIZE_FORMAT ": %s\n",
+				sig, written, sizeof(int), g_strerror(errno));
+		exit(1);
+	}
+}
+
+static gboolean
+mainloop_sighandler(GIOChannel *source, GIOCondition cond, gpointer data)
 {
+	GIOStatus stat;
+	int sig;
+	gsize bytes_read;
+	GError *error = NULL;
+
+	/* read the signal number off of the io channel */
+	stat = g_io_channel_read_chars(source, (gchar *)&sig, sizeof(int),
+			&bytes_read, &error);
+	if (stat != G_IO_STATUS_NORMAL) {
+		purple_debug_error("sighandler", "Signal callback failed to read "
+				"from signal socket: %s", error->message);
+		purple_core_quit();
+		return FALSE;
+	}
+
 	switch (sig) {
 	case SIGHUP:
 		purple_debug_warning("sighandler", "Caught signal %d\n", sig);
 		break;
-	case SIGSEGV:
-		fprintf(stderr, "%s", segfault_message);
-		abort();
-		break;
 #if defined(USE_GSTREAMER) && !defined(GST_CAN_DISABLE_FORKING)
 /* By default, gstreamer forks when you initialize it, and waitpids for the
  * child.  But if libpurple reaps the child rather than leaving it to
@@ -219,6 +252,8 @@
 		purple_debug_warning("sighandler", "Caught signal %d\n", sig);
 		purple_core_quit();
 	}
+
+	return TRUE;
 }
 #endif
 
@@ -502,6 +537,8 @@
 	sigset_t sigset;
 	RETSIGTYPE (*prev_sig_disp)(int);
 	char errmsg[BUFSIZ];
+	GIOChannel *signal_channel;
+	GIOStatus signal_status;
 #ifndef DEBUG
 	char *segfault_message_tmp;
 	GError *error = NULL;
@@ -592,6 +629,29 @@
 		);
 #endif
 
+	/*
+	 * Create a socket pair for receiving unix signals from a signal
+	 * handler.
+	 */
+	if (socketpair(AF_UNIX, SOCK_STREAM, 0, signal_sockets) < 0) {
+		perror("Failed to create sockets for GLib signal handling");
+		exit(1);
+	}
+	signal_channel = g_io_channel_unix_new(signal_sockets[1]);
+
+	/*
+	 * Set the channel encoding to raw binary instead of the default of
+	 * UTF-8, because we'll be sending integers across instead of strings.
+	 */
+	error = NULL;
+	signal_status = g_io_channel_set_encoding(signal_channel, NULL, &error);
+	if (signal_status != G_IO_STATUS_NORMAL) {
+		fprintf(stderr, "Failed to set the signal channel to raw "
+				"binary: %s", error->message);
+		exit(1);
+	}
+	g_io_add_watch(signal_channel, G_IO_IN, mainloop_sighandler, NULL);
+
 	/* Let's not violate any PLA's!!!! */
 	/* jseymour: whatever the fsck that means */
 	/* Robot101: for some reason things like gdm like to block     *