ctdb-common: Start listening to sockets only on successful startup

Fix tests to use wait_send() instead of startup() as a synchronization
point to ensure that the socket is listening.

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

3 files changed, 161 insertions, 43 deletions

diff --git a/ctdb/common/sock_daemon.c b/ctdb/common/sock_daemon.c
index 8ceb7c96bda..ae291d773c3 100644
--- a/ctdb/common/sock_daemon.c
+++ b/ctdb/common/sock_daemon.c
@@ -524,6 +524,7 @@ static void sock_daemon_run_signal_handler(struct tevent_context *ev,
 					   void *private_data);
 static void sock_daemon_run_reconfigure(struct tevent_req *req);
 static void sock_daemon_run_shutdown(struct tevent_req *req);
+static bool sock_daemon_run_socket_listen(struct tevent_req *req);
 static void sock_daemon_run_socket_fail(struct tevent_req *subreq);
 static void sock_daemon_run_watch_pid(struct tevent_req *subreq);
 static void sock_daemon_run_wait(struct tevent_req *req);
@@ -539,8 +540,6 @@ struct tevent_req *sock_daemon_run_send(TALLOC_CTX *mem_ctx,
 	struct tevent_req *req, *subreq;
 	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);
@@ -557,7 +556,6 @@ 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;
@@ -596,16 +594,6 @@ struct tevent_req *sock_daemon_run_send(TALLOC_CTX *mem_ctx,
 		return tevent_req_post(req, ev);
 	}
 
-	for (sock = sockd->socket_list; sock != NULL; sock = sock->next) {
-		subreq = sock_socket_start_send(state, ev, sock,
-						remove_before_use);
-		if (tevent_req_nomem(subreq, req)) {
-			return tevent_req_post(req, ev);
-		}
-		tevent_req_set_callback(subreq, sock_daemon_run_socket_fail,
-					req);
-	}
-
 	if (pid_watch > 1) {
 		subreq = tevent_wakeup_send(state, ev,
 					    tevent_timeval_current_ofs(1,0));
@@ -650,6 +638,10 @@ static void sock_daemon_run_started(struct tevent_req *subreq)
 		D_NOTICE("startup completed successfully\n");
 	}
 
+	status = sock_daemon_run_socket_listen(req);
+	if (! status) {
+		return;
+	}
 	sock_daemon_run_wait(req);
 }
 
@@ -714,6 +706,31 @@ static void sock_daemon_run_shutdown(struct tevent_req *req)
 	TALLOC_FREE(sockd->pid_ctx);
 }
 
+static bool sock_daemon_run_socket_listen(struct tevent_req *req)
+{
+	struct tevent_req *subreq;
+	struct sock_daemon_run_state *state = tevent_req_data(
+		req, struct sock_daemon_run_state);
+	struct sock_daemon_context *sockd = state->sockd;
+	struct sock_socket *sock;
+	bool remove_before_use = false;
+
+	if (sockd->pid_ctx != NULL) {
+		remove_before_use = true;
+	}
+	for (sock = sockd->socket_list; sock != NULL; sock = sock->next) {
+		subreq = sock_socket_start_send(state, state->ev, sock,
+						remove_before_use);
+		if (tevent_req_nomem(subreq, req)) {
+			return false;
+		}
+		tevent_req_set_callback(subreq, sock_daemon_run_socket_fail,
+					req);
+	}
+
+	return true;
+}
+
 static void sock_daemon_run_socket_fail(struct tevent_req *subreq)
 {
 	struct tevent_req *req = tevent_req_callback_data(
diff --git a/ctdb/tests/cunit/sock_daemon_test_001.sh b/ctdb/tests/cunit/sock_daemon_test_001.sh
index 0561863e256..bf925fbe2ab 100755
--- a/ctdb/tests/cunit/sock_daemon_test_001.sh
+++ b/ctdb/tests/cunit/sock_daemon_test_001.sh
@@ -25,16 +25,17 @@ result_filter ()
 ok <<EOF
 test1[PID]: daemon started, pid=PID
 test1[PID]: startup failed, ret=1
-test1[PID]: listening on $sockpath
 test1[PID]: daemon started, pid=PID
+test1[PID]: startup completed successfully
+test1[PID]: listening on $sockpath
 test1[PID]: Shutting down
 EOF
 unit_test sock_daemon_test "$pidfile" "$sockpath" 1
 
 ok <<EOF
-test2[PID]: listening on $sockpath
 test2[PID]: daemon started, pid=PID
 test2[PID]: startup completed successfully
+test2[PID]: listening on $sockpath
 test2[PID]: Received signal 1
 test2[PID]: reconfigure failed, ret=1
 test2[PID]: Received signal 10
@@ -45,8 +46,8 @@ EOF
 unit_test sock_daemon_test "$pidfile" "$sockpath" 2
 
 ok <<EOF
-test3[PID]: listening on $sockpath
 test3[PID]: daemon started, pid=PID
+test3[PID]: listening on $sockpath
 test3[PID]: PID PID gone away, exiting
 test3[PID]: Shutting down
 EOF
@@ -59,18 +60,16 @@ EOF
 unit_test sock_daemon_test "$pidfile" "$sockpath" 4
 
 ok <<EOF
-test5[PID]: listening on $sockpath
 test5[PID]: daemon started, pid=PID
-test5[PID]: startup completed successfully
+test5[PID]: listening on $sockpath
 test5[PID]: Received signal 15
 test5[PID]: Shutting down
 EOF
 unit_test sock_daemon_test "$pidfile" "$sockpath" 5
 
 ok <<EOF
-test6[PID]: listening on $sockpath
 test6[PID]: daemon started, pid=PID
-test6[PID]: startup completed successfully
+test6[PID]: listening on $sockpath
 test6[PID]: Shutting down
 EOF
 unit_test sock_daemon_test "$pidfile" "$sockpath" 6
@@ -108,12 +107,10 @@ EOF
 unit_test sock_daemon_test "$pidfile" "$sockpath" 9
 
 ok <<EOF
-test10[PID]: listening on $sockpath
 test10[PID]: daemon started, pid=PID
-test10[PID]: startup completed successfully
 test10[PID]: listening on $sockpath
 test10[PID]: daemon started, pid=PID
-test10[PID]: startup completed successfully
+test10[PID]: listening on $sockpath
 test10[PID]: Received signal 15
 test10[PID]: Shutting down
 EOF
diff --git a/ctdb/tests/src/sock_daemon_test.c b/ctdb/tests/src/sock_daemon_test.c
index 82f4f783a1e..df86a73150a 100644
--- a/ctdb/tests/src/sock_daemon_test.c
+++ b/ctdb/tests/src/sock_daemon_test.c
@@ -68,6 +68,18 @@ static int test1_startup_fail(void *private_data)
 	return 1;
 }
 
+static int test1_startup(void *private_data)
+{
+	const char *sockpath = (const char *)private_data;
+	struct stat st;
+	int ret;
+
+	ret = stat(sockpath, &st);
+	assert(ret == -1);
+
+	return 0;
+}
+
 static struct tevent_req *dummy_read_send(TALLOC_CTX *mem_ctx,
 					  struct tevent_context *ev,
 					  struct sock_client_context *client,
@@ -125,6 +137,7 @@ static void test1(TALLOC_CTX *mem_ctx, const char *pidfile,
 	talloc_free(sockd);
 
 	test1_funcs = (struct sock_daemon_funcs){
+		.startup = test1_startup,
 		.wait_send = dummy_wait_send,
 		.wait_recv = dummy_wait_recv,
 	};
@@ -677,8 +690,15 @@ static struct sock_socket_funcs test5_client_funcs = {
 	.read_recv = test5_read_recv,
 };
 
-static int test5_startup(void *private_data)
+struct test5_wait_state {
+};
+
+static struct tevent_req *test5_wait_send(TALLOC_CTX *mem_ctx,
+					  struct tevent_context *ev,
+					  void *private_data)
 {
+	struct tevent_req *req;
+	struct test5_wait_state *state;
 	int fd = *(int *)private_data;
 	int ret = 1;
 	ssize_t nwritten;
@@ -686,11 +706,23 @@ static int test5_startup(void *private_data)
 	nwritten = write(fd, &ret, sizeof(ret));
 	assert(nwritten == sizeof(ret));
 	close(fd);
-	return 0;
+
+	req = tevent_req_create(mem_ctx, &state, struct test5_wait_state);
+	if (req == NULL) {
+		return NULL;
+	}
+
+	return req;
+}
+
+static bool test5_wait_recv(struct tevent_req *req, int *perr)
+{
+	return true;
 }
 
 static struct sock_daemon_funcs test5_funcs = {
-	.startup = test5_startup,
+	.wait_send = test5_wait_send,
+	.wait_recv = test5_wait_recv,
 };
 
 static void test5(TALLOC_CTX *mem_ctx, const char *pidfile,
@@ -910,20 +942,6 @@ static struct sock_socket_funcs test6_client_funcs = {
 	.read_recv = test6_read_recv,
 };
 
-static int test6_startup(void *private_data)
-{
-	struct test6_server_state *server_state =
-		(struct test6_server_state *)private_data;
-	int ret = 1;
-	ssize_t nwritten;
-
-	nwritten = write(server_state->fd, &ret, sizeof(ret));
-	assert(nwritten == sizeof(ret));
-	close(server_state->fd);
-	server_state->fd = -1;
-	return 0;
-}
-
 struct test6_wait_state {
 	struct test6_server_state *server_state;
 };
@@ -934,8 +952,17 @@ static struct tevent_req *test6_wait_send(TALLOC_CTX *mem_ctx,
 					  struct tevent_context *ev,
 					  void *private_data)
 {
+	struct test6_server_state *server_state =
+		(struct test6_server_state *)private_data;
 	struct tevent_req *req, *subreq;
 	struct test6_wait_state *state;
+	ssize_t nwritten;
+	int ret = 1;
+
+	nwritten = write(server_state->fd, &ret, sizeof(ret));
+	assert(nwritten == sizeof(ret));
+	close(server_state->fd);
+	server_state->fd = -1;
 
 	req = tevent_req_create(mem_ctx, &state, struct test6_wait_state);
 	if (req == NULL) {
@@ -992,7 +1019,6 @@ static bool test6_wait_recv(struct tevent_req *req, int *perr)
 }
 
 static struct sock_daemon_funcs test6_funcs = {
-	.startup = test6_startup,
 	.wait_send = test6_wait_send,
 	.wait_recv = test6_wait_recv,
 };
@@ -1366,6 +1392,84 @@ static void test9(TALLOC_CTX *mem_ctx, const char *pidfile,
 	close(fd[0]);
 }
 
+static void test10_shutdown(void *private_data)
+{
+	int fd = *(int *)private_data;
+	int ret = 3;
+	ssize_t nwritten;
+
+	nwritten = write(fd, &ret, sizeof(ret));
+	assert(nwritten == sizeof(ret));
+}
+
+struct test10_wait_state {
+};
+
+static void test10_wait_done(struct tevent_req *subreq);
+
+static struct tevent_req *test10_wait_send(TALLOC_CTX *mem_ctx,
+					   struct tevent_context *ev,
+					   void *private_data)
+{
+	int fd = *(int *)private_data;
+	struct tevent_req *req, *subreq;
+	struct test10_wait_state *state;
+	size_t nwritten;
+	int ret = 1;
+
+	req = tevent_req_create(mem_ctx, &state, struct test10_wait_state);
+	if (req == NULL) {
+		return NULL;
+	}
+
+	subreq = tevent_wakeup_send(state, ev,
+				    tevent_timeval_current_ofs(10, 0));
+	if (tevent_req_nomem(subreq, req)) {
+		return tevent_req_post(req, ev);
+	}
+	tevent_req_set_callback(subreq, test10_wait_done, req);
+
+	nwritten = write(fd, &ret, sizeof(ret));
+	assert(nwritten == sizeof(ret));
+
+	return req;
+}
+
+static void test10_wait_done(struct tevent_req *subreq)
+{
+	struct tevent_req *req = tevent_req_callback_data(
+		subreq, struct tevent_req);
+	bool status;
+
+	status = tevent_wakeup_recv(subreq);
+	if (! status) {
+		tevent_req_error(req, EIO);
+		return;
+	}
+
+	tevent_req_done(req);
+}
+
+static bool test10_wait_recv(struct tevent_req *req, int *perr)
+{
+	int ret;
+
+	if (tevent_req_is_unix_error(req, &ret)) {
+		if (perr != NULL) {
+			*perr = ret;
+		}
+		return false;
+	}
+
+	return true;
+}
+
+static struct sock_daemon_funcs test10_funcs = {
+	.shutdown = test10_shutdown,
+	.wait_send = test10_wait_send,
+	.wait_recv = test10_wait_recv,
+};
+
 /*
  * test10
  *
@@ -1399,7 +1503,7 @@ static void test10(TALLOC_CTX *mem_ctx, const char *pidfile,
 		assert(ev != NULL);
 
 		ret = sock_daemon_setup(mem_ctx, "test10", "file:", "NOTICE",
-					&test2_funcs, &fd[1], &sockd);
+					&test10_funcs, &fd[1], &sockd);
 		assert(ret == 0);
 
 		ret = sock_daemon_add_unix(sockd, sockpath,
@@ -1443,7 +1547,7 @@ static void test10(TALLOC_CTX *mem_ctx, const char *pidfile,
 		assert(ev != NULL);
 
 		ret = sock_daemon_setup(mem_ctx, "test10", "file:", "NOTICE",
-					&test2_funcs, &fd[1], &sockd);
+					&test10_funcs, &fd[1], &sockd);
 		assert(ret == 0);
 
 		ret = sock_daemon_add_unix(sockd, sockpath,