From 2ea5a2e38a864509d8edef0cb179a05f686f58be Mon Sep 17 00:00:00 2001
From: Alex Young <alex@bytemark.co.uk>
Date: Fri, 13 Jul 2012 14:09:52 +0100
Subject: [PATCH] Unlink the control socket on clean shutdown

Previously, the behaviour was to unlink any control socket sat where we
wanted to open ours.  This would make us lose control of running servers
if we happened to collide accidentally.  With this patch, the new
process will abort() if there is a control socket squatting on the
path we want, and unlink it when it closes.

This means that an unclean shutdown will leave a dangling, unattached
control socket which will block a restart, but that's a better option
than intentionally cutting off running servers.
---
 src/control.c | 26 ++++++++++++++++++++------
 src/util.c    | 15 +++++++--------
 2 files changed, 27 insertions(+), 14 deletions(-)

diff --git a/src/control.c b/src/control.c
index dd93205..d670018 100644
--- a/src/control.c
+++ b/src/control.c
@@ -172,12 +172,12 @@ int open_control_socket( const char * socket_name )
 	bind_address.sun_family = AF_UNIX;
 	strncpy(bind_address.sun_path, socket_name, sizeof(bind_address.sun_path)-1);
 	
-	unlink(socket_name); /* ignore failure */
+	//unlink(socket_name); /* ignore failure */
 	
 	FATAL_IF_NEGATIVE(
 		bind(control_fd , &bind_address, sizeof(bind_address)),
-		"Couldn't bind control socket to %s",
-		socket_name
+		"Couldn't bind control socket to %s: %s",
+		socket_name, strerror( errno )
 	);
 	
 	FATAL_IF_NEGATIVE(
@@ -203,13 +203,27 @@ void control_serve( struct control * control )
 }
 
 
+void control_cleanup( 
+		struct control * control, 
+		int fatal __attribute__((unused)) )
+{
+	NULLCHECK( control );
+	unlink( control->socket_name );
+	close( control->control_fd );
+}
+
+
 void * control_runner( void * control_uncast )
 {
 	debug("Control thread");
 	NULLCHECK( control_uncast );
 	struct control * control = (struct control *)control_uncast;
 
+	error_set_handler( (cleanup_handler*)control_cleanup, control );
+
 	control_serve( control );
+
+	control_cleanup( control, 0 );
 	return NULL;
 }
 
@@ -390,7 +404,7 @@ int control_status(
 	return 0;
 }
 
-void control_cleanup(struct control_client* client, 
+void control_client_cleanup(struct control_client* client, 
 		int fatal __attribute__ ((unused)) )
 {
 	if (client->socket) { close(client->socket); }
@@ -408,7 +422,7 @@ void control_respond(struct control_client * client)
 {
 	char **lines = NULL;
 
-	error_set_handler((cleanup_handler*) control_cleanup, client);
+	error_set_handler((cleanup_handler*) control_client_cleanup, client);
 
 	int i, linesc;		
 	linesc = read_lines_until_blankline(client->socket, 256, &lines);
@@ -445,7 +459,7 @@ void control_respond(struct control_client * client)
 	}
 	free(lines);
 
-	control_cleanup(client, 0);
+	control_client_cleanup(client, 0);
 	debug("control command handled" );
 }
 
diff --git a/src/util.c b/src/util.c
index 2c44e86..aff72be 100644
--- a/src/util.c
+++ b/src/util.c
@@ -18,18 +18,17 @@ void error_init(void)
 	pthread_key_create(&cleanup_handler_key, free);
 }
 
-void error_handler(int fatal __attribute__ ((unused)) )
+void error_handler(int fatal)
 {
 	DECLARE_ERROR_CONTEXT(context);
 	
-	if (!context) {
-		/* FIXME: This can't be right - by default we exit()
-		 * with a status of 0 in this case.
-		 */
-		pthread_exit((void*) 1);
+	if (context) {
+		longjmp(context->jmp, fatal ? 1 : 2 );
+	} 
+	else {
+		if ( fatal ) { abort(); }
+		else { pthread_exit((void*) 1); }
 	}
-		
-	longjmp(context->jmp, fatal ? 1 : 2 );
 }