ctdb-common: Fix stale socket removal

Sockets need to be created from sock_daemon_run_send().  This means
that stale socket removal can depend on the PID file context being
initialised.

Also fix associated test.

Signed-off-by: Amitay Isaacs <amitay@gmail.com>
Reviewed-by: Martin Schwenke <martin@meltin.net>

3 files changed, 61 insertions, 23 deletions

diff --git a/ctdb/common/sock_daemon.c b/ctdb/common/sock_daemon.c
index 56205d019ec..ba171af52c9 100644
--- a/ctdb/common/sock_daemon.c
+++ b/ctdb/common/sock_daemon.c
@@ -239,6 +239,8 @@ static int socket_setup(const char *sockpath, bool remove_before_use)
 		return -1;
 	}
 
+	D_NOTICE("listening on %s\n", sockpath);
+
 	return fd;
 }
 
@@ -247,7 +249,6 @@ static int sock_socket_destructor(struct sock_socket *sock);
 static int sock_socket_init(TALLOC_CTX *mem_ctx, const char *sockpath,
 			    struct sock_socket_funcs *funcs,
 			    void *private_data,
-			    bool remove_before_use,
 			    struct sock_socket **result)
 {
 	struct sock_socket *sock;
@@ -267,12 +268,7 @@ static int sock_socket_init(TALLOC_CTX *mem_ctx, const char *sockpath,
 	sock->sockpath = sockpath;
 	sock->funcs = funcs;
 	sock->private_data = private_data;
-
-	sock->fd = socket_setup(sockpath, remove_before_use);
-	if (sock->fd == -1) {
-		talloc_free(sock);
-		return EIO;
-	}
+	sock->fd = -1;
 
 	talloc_set_destructor(sock, sock_socket_destructor);
 
@@ -306,7 +302,8 @@ static int sock_socket_start_client_destructor(struct sock_client *client);
 
 static struct tevent_req *sock_socket_start_send(TALLOC_CTX *mem_ctx,
 						 struct tevent_context *ev,
-						 struct sock_socket *sock)
+						 struct sock_socket *sock,
+						 bool remove_before_use)
 {
 	struct tevent_req *req, *subreq;
 	struct sock_socket_start_state *state;
@@ -320,6 +317,12 @@ static struct tevent_req *sock_socket_start_send(TALLOC_CTX *mem_ctx,
 	state->ev = ev;
 	state->sock = sock;
 
+	sock->fd = socket_setup(sock->sockpath, remove_before_use);
+	if (sock->fd == -1) {
+		tevent_req_error(req, EIO);
+		return tevent_req_post(req, ev);
+	}
+
 	talloc_set_destructor(state, sock_socket_start_state_destructor);
 
 	subreq = accept_send(state, ev, sock->fd);
@@ -487,17 +490,12 @@ int sock_daemon_add_unix(struct sock_daemon_context *sockd,
 {
 	struct sock_socket *sock;
 	int ret;
-	bool remove_before_use = false;
 
-	remove_before_use = (sockd->pid_ctx != NULL) ? true : false;
-
-	ret = sock_socket_init(sockd, sockpath, funcs, private_data,
-			       remove_before_use, &sock);
+	ret = sock_socket_init(sockd, sockpath, funcs, private_data, &sock);
 	if (ret != 0) {
 		return ret;
 	}
 
-	D_NOTICE("listening on %s\n", sockpath);
 
 	DLIST_ADD(sockd->socket_list, sock);
 	return 0;
@@ -537,6 +535,7 @@ struct tevent_req *sock_daemon_run_send(TALLOC_CTX *mem_ctx,
 	struct sock_daemon_run_state *state;
 	struct tevent_signal *se;
 	struct sock_socket *sock;
+	bool remove_before_use = false;
 
 	req = tevent_req_create(mem_ctx, &state,
 				struct sock_daemon_run_state);
@@ -553,6 +552,7 @@ struct tevent_req *sock_daemon_run_send(TALLOC_CTX *mem_ctx,
 			tevent_req_error(req, EEXIST);
 			return tevent_req_post(req, ev);
 		}
+		remove_before_use = true;
 	}
 
 	state->ev = ev;
@@ -592,7 +592,8 @@ struct tevent_req *sock_daemon_run_send(TALLOC_CTX *mem_ctx,
 	}
 
 	for (sock = sockd->socket_list; sock != NULL; sock = sock->next) {
-		subreq = sock_socket_start_send(state, ev, sock);
+		subreq = sock_socket_start_send(state, ev, sock,
+						remove_before_use);
 		if (tevent_req_nomem(subreq, req)) {
 			return tevent_req_post(req, ev);
 		}
diff --git a/ctdb/tests/cunit/sock_daemon_test_001.sh b/ctdb/tests/cunit/sock_daemon_test_001.sh
index 1d2607f36ea..58742755d0d 100755
--- a/ctdb/tests/cunit/sock_daemon_test_001.sh
+++ b/ctdb/tests/cunit/sock_daemon_test_001.sh
@@ -24,6 +24,7 @@ result_filter ()
 
 ok <<EOF
 test1[PID]: listening on $sockpath
+test1[PID]: Shutting down
 EOF
 unit_test sock_daemon_test "$pidfile" "$sockpath" 1
 
diff --git a/ctdb/tests/src/sock_daemon_test.c b/ctdb/tests/src/sock_daemon_test.c
index 95b0909b78e..bbb792753e1 100644
--- a/ctdb/tests/src/sock_daemon_test.c
+++ b/ctdb/tests/src/sock_daemon_test.c
@@ -32,6 +32,42 @@
 #include "common/sock_daemon.c"
 #include "common/sock_io.c"
 
+struct dummy_wait_state {
+};
+
+static struct tevent_req *dummy_wait_send(TALLOC_CTX *mem_ctx,
+					  struct tevent_context *ev,
+					  void *private_data)
+{
+	struct tevent_req *req;
+	struct dummy_wait_state *state;
+	const char *sockpath = (const char *)private_data;
+	struct stat st;
+	int ret;
+
+	ret = stat(sockpath, &st);
+	assert(ret == 0);
+	assert(S_ISSOCK(st.st_mode));
+
+	req = tevent_req_create(mem_ctx, &state, struct dummy_wait_state);
+	if (req == NULL) {
+		return NULL;
+	}
+
+	tevent_req_done(req);
+	return tevent_req_post(req, ev);
+}
+
+static bool dummy_wait_recv(struct tevent_req *req, int *perr)
+{
+	return true;
+}
+
+static struct sock_daemon_funcs test1_funcs = {
+	.wait_send = dummy_wait_send,
+	.wait_recv = dummy_wait_recv,
+};
+
 static struct tevent_req *dummy_read_send(TALLOC_CTX *mem_ctx,
 					  struct tevent_context *ev,
 					  struct sock_client_context *client,
@@ -63,12 +99,13 @@ static struct sock_socket_funcs dummy_socket_funcs = {
 static void test1(TALLOC_CTX *mem_ctx, const char *pidfile,
 		  const char *sockpath)
 {
+	struct tevent_context *ev;
 	struct sock_daemon_context *sockd;
 	struct stat st;
 	int ret;
 
 	ret = sock_daemon_setup(mem_ctx, "test1", "file:", "NOTICE",
-				NULL, NULL, &sockd);
+				&test1_funcs, discard_const(sockpath), &sockd);
 	assert(ret == 0);
 	assert(sockd != NULL);
 
@@ -79,16 +116,15 @@ static void test1(TALLOC_CTX *mem_ctx, const char *pidfile,
 	assert(ret == 0);
 
 	ret = stat(sockpath, &st);
-	assert(ret == 0);
-	assert(S_ISSOCK(st.st_mode));
+	assert(ret == -1);
 
-	talloc_free(sockd);
+	ev = tevent_context_init(mem_ctx);
+	assert(ev != NULL);
 
-	ret = stat(pidfile, &st);
-	assert(ret == -1);
+	ret = sock_daemon_run(ev, sockd, NULL, false, false, -1);
+	assert(ret == 0);
 
-	ret = stat(sockpath, &st);
-	assert(ret == -1);
+	talloc_free(mem_ctx);
 }
 
 /*