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>

diff --git a/ctdb/common/sock_daemon.c b/ctdb/common/sock_daemon.c
index 56205d019ecc4c2980c8a9336c216d6699081df5..ba171af52c9acc1f4186f8927f42b8b3eb16d9f7 100644 (file)
--- 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 1d2607f36ea1ed9e874947492aa5f7a00a663cd9..58742755d0d558c5545491c2fac5b77355fd47c8 100755 (executable)
--- 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 95b0909b78e8f614c973aea9bb84a8f80e6b3274..bbb792753e1fd012b1979435970918f44a0241b2 100644 (file)
--- 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);
 }
 
 /*