From 7b4d0364d9797848f4ad3a74aeb6effea9f80a2b Mon Sep 17 00:00:00 2001 From: Amitay Isaacs Date: Mon, 19 Sep 2016 16:30:12 +1000 Subject: [PATCH 1/6] ctdb-common: Add routines to manage PID file BUG: https://bugzilla.samba.org/show_bug.cgi?id=12287 Signed-off-by: Amitay Isaacs Reviewed-by: Martin Schwenke (cherry picked from commit 97b6ac7f662d8de316ed520e038779e79bcdb7bc) --- ctdb/common/pidfile.c | 143 +++++++++++++++++++++ ctdb/common/pidfile.h | 51 ++++++++ ctdb/tests/cunit/pidfile_test_001.sh | 8 ++ ctdb/tests/src/ctdbd_test.c | 1 + ctdb/tests/src/pidfile_test.c | 241 +++++++++++++++++++++++++++++++++++ ctdb/wscript | 3 +- 6 files changed, 446 insertions(+), 1 deletion(-) create mode 100644 ctdb/common/pidfile.c create mode 100644 ctdb/common/pidfile.h create mode 100755 ctdb/tests/cunit/pidfile_test_001.sh create mode 100644 ctdb/tests/src/pidfile_test.c diff --git a/ctdb/common/pidfile.c b/ctdb/common/pidfile.c new file mode 100644 index 0000000..b3f29e3 --- /dev/null +++ b/ctdb/common/pidfile.c @@ -0,0 +1,143 @@ +/* + Create and remove pidfile + + Copyright (C) Amitay Isaacs 2016 + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program; if not, see . +*/ + +#include "replace.h" +#include "system/filesys.h" + +#include + +#include "common/pidfile.h" + +struct pidfile_context { + const char *pidfile; + int fd; + pid_t pid; +}; + +static int pidfile_context_destructor(struct pidfile_context *pid_ctx); + +int pidfile_create(TALLOC_CTX *mem_ctx, const char *pidfile, + struct pidfile_context **result) +{ + struct pidfile_context *pid_ctx; + struct flock lck; + char tmp[64]; + int fd, ret = 0; + int len; + ssize_t nwritten; + + pid_ctx = talloc_zero(mem_ctx, struct pidfile_context); + if (pid_ctx == NULL) { + return ENOMEM; + } + + pid_ctx->pidfile = talloc_strdup(pid_ctx, pidfile); + if (pid_ctx->pidfile == NULL) { + ret = ENOMEM; + goto fail; + } + + pid_ctx->pid = getpid(); + + fd = open(pidfile, O_CREAT|O_WRONLY|O_NONBLOCK, 0644); + if (fd == -1) { + ret = errno; + goto fail; + } + + pid_ctx->fd = fd; + + lck = (struct flock) { + .l_type = F_WRLCK, + .l_whence = SEEK_SET, + }; + + do { + ret = fcntl(fd, F_SETLK, &lck); + } while ((ret == -1) && (errno == EINTR)); + + if (ret != 0) { + ret = errno; + goto fail; + } + + do { + ret = ftruncate(fd, 0); + } while ((ret == -1) && (errno == EINTR)); + + if (ret == -1) { + ret = EIO; + goto fail_unlink; + } + + len = snprintf(tmp, sizeof(tmp), "%u\n", pid_ctx->pid); + if (len < 0) { + ret = EIO; + goto fail_unlink; + } + + do { + nwritten = write(fd, tmp, len); + } while ((nwritten == -1) && (errno == EINTR)); + + if ((nwritten == -1) || (nwritten != len)) { + ret = EIO; + goto fail_unlink; + } + + talloc_set_destructor(pid_ctx, pidfile_context_destructor); + + *result = pid_ctx; + return 0; + +fail_unlink: + unlink(pidfile); + close(fd); + +fail: + talloc_free(pid_ctx); + return ret; +} + +static int pidfile_context_destructor(struct pidfile_context *pid_ctx) +{ + struct flock lck; + int ret; + + if (getpid() != pid_ctx->pid) { + return 0; + } + + lck = (struct flock) { + .l_type = F_UNLCK, + .l_whence = SEEK_SET, + }; + + (void) unlink(pid_ctx->pidfile); + + do { + ret = fcntl(pid_ctx->fd, F_SETLK, &lck); + } while ((ret == -1) && (errno == EINTR)); + + do { + ret = close(pid_ctx->fd); + } while ((ret == -1) && (errno == EINTR)); + + return 0; +} diff --git a/ctdb/common/pidfile.h b/ctdb/common/pidfile.h new file mode 100644 index 0000000..1450134 --- /dev/null +++ b/ctdb/common/pidfile.h @@ -0,0 +1,51 @@ +/* + Create and remove pidfile + + Copyright (C) Amitay Isaacs 2016 + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program; if not, see . +*/ + +#ifndef __CTDB_PIDFILE_H__ +#define __CTDB_PIDFILE_H__ + +#include + +/** + * @file pidfile.h + * + * @brief Routines to manage PID file + */ + +/** + * @brief Abstract struct to store pidfile details + */ +struct pidfile_context; + +/** + * @brief Create a PID file + * + * This creates a PID file, locks it, and writes PID. + * + * @param[in] mem_ctx Talloc memory context + * @param[in] pidfile Path of PID file + * @param[out] result Pidfile context + * @return 0 on success, errno on failure + * + * Freeing the pidfile_context, will delete the pidfile. + */ +int pidfile_create(TALLOC_CTX *mem_ctx, const char *pidfile, + struct pidfile_context **result); + +#endif /* __CTDB_PIDFILE_H__ */ diff --git a/ctdb/tests/cunit/pidfile_test_001.sh b/ctdb/tests/cunit/pidfile_test_001.sh new file mode 100755 index 0000000..620682e --- /dev/null +++ b/ctdb/tests/cunit/pidfile_test_001.sh @@ -0,0 +1,8 @@ +#!/bin/sh + +. "${TEST_SCRIPTS_DIR}/unit.sh" + +pidfile=$(mktemp --tmpdir="$TEST_VAR_DIR") + +ok_null +unit_test pidfile_test $pidfile diff --git a/ctdb/tests/src/ctdbd_test.c b/ctdb/tests/src/ctdbd_test.c index 018aa2a..e66a29b 100644 --- a/ctdb/tests/src/ctdbd_test.c +++ b/ctdb/tests/src/ctdbd_test.c @@ -47,6 +47,7 @@ bool fast_start; #include "common/rb_tree.c" #include "common/reqid.c" #include "common/logging.c" +#include "common/pidfile.c" /* CTDB_SERVER_OBJ */ #include "server/ctdb_daemon.c" diff --git a/ctdb/tests/src/pidfile_test.c b/ctdb/tests/src/pidfile_test.c new file mode 100644 index 0000000..ad8bf14 --- /dev/null +++ b/ctdb/tests/src/pidfile_test.c @@ -0,0 +1,241 @@ +/* + pidfile tests + + Copyright (C) Amitay Isaacs 2016 + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program; if not, see . +*/ + +#include "replace.h" +#include "system/wait.h" + +#include + +#include "common/pidfile.c" + + +/* create pid file, check pid file exists, check pid and remove pid file */ +static void test1(const char *pidfile) +{ + struct pidfile_context *pid_ctx; + int ret; + struct stat st; + FILE *fp; + pid_t pid; + + ret = pidfile_create(NULL, pidfile, &pid_ctx); + assert(ret == 0); + assert(pid_ctx != NULL); + + ret = stat(pidfile, &st); + assert(ret == 0); + assert(S_ISREG(st.st_mode)); + + fp = fopen(pidfile, "r"); + ret = fscanf(fp, "%d", &pid); + assert(ret == 1); + assert(pid == getpid()); + fclose(fp); + + TALLOC_FREE(pid_ctx); + + ret = stat(pidfile, &st); + assert(ret == -1); +} + +/* create pid file in two processes */ +static void test2(const char *pidfile) +{ + struct pidfile_context *pid_ctx; + pid_t pid, pid2; + int fd[2]; + int ret; + size_t nread; + FILE *fp; + struct stat st; + + ret = pipe(fd); + assert(ret == 0); + + pid = fork(); + assert(pid != -1); + + if (pid == 0) { + ssize_t nwritten; + + close(fd[0]); + + ret = pidfile_create(NULL, pidfile, &pid_ctx); + assert(ret == 0); + assert(pid_ctx != NULL); + + nwritten = write(fd[1], &ret, sizeof(ret)); + assert(nwritten == sizeof(ret)); + + sleep(10); + + TALLOC_FREE(pid_ctx); + + nwritten = write(fd[1], &ret, sizeof(ret)); + assert(nwritten == sizeof(ret)); + + exit(1); + } + + close(fd[1]); + + nread = read(fd[0], &ret, sizeof(ret)); + assert(nread == sizeof(ret)); + assert(ret == 0); + + fp = fopen(pidfile, "r"); + assert(fp != NULL); + ret = fscanf(fp, "%d", &pid2); + assert(ret == 1); + assert(pid == pid2); + fclose(fp); + + ret = pidfile_create(NULL, pidfile, &pid_ctx); + assert(ret != 0); + + nread = read(fd[0], &ret, sizeof(ret)); + assert(nread == sizeof(ret)); + assert(ret == 0); + + ret = pidfile_create(NULL, pidfile, &pid_ctx); + assert(ret == 0); + assert(pid_ctx != NULL); + + TALLOC_FREE(pid_ctx); + + ret = stat(pidfile, &st); + assert(ret == -1); +} + +/* create pid file, fork, try to remove pid file in separate process */ +static void test3(const char *pidfile) +{ + struct pidfile_context *pid_ctx; + pid_t pid; + int fd[2]; + int ret; + size_t nread; + struct stat st; + + ret = pidfile_create(NULL, pidfile, &pid_ctx); + assert(ret == 0); + assert(pid_ctx != NULL); + + ret = pipe(fd); + assert(ret == 0); + + pid = fork(); + assert(pid != -1); + + if (pid == 0) { + ssize_t nwritten; + + close(fd[0]); + + TALLOC_FREE(pid_ctx); + + nwritten = write(fd[1], &ret, sizeof(ret)); + assert(nwritten == sizeof(ret)); + + exit(1); + } + + close(fd[1]); + + nread = read(fd[0], &ret, sizeof(ret)); + assert(nread == sizeof(ret)); + + ret = stat(pidfile, &st); + assert(ret == 0); + + TALLOC_FREE(pid_ctx); + + ret = stat(pidfile, &st); + assert(ret == -1); +} + +/* create pid file, kill process, overwrite pid file in different process */ +static void test4(const char *pidfile) +{ + struct pidfile_context *pid_ctx; + pid_t pid, pid2; + int fd[2]; + int ret; + size_t nread; + struct stat st; + + ret = pipe(fd); + assert(ret == 0); + + pid = fork(); + assert(pid != -1); + + if (pid == 0) { + ssize_t nwritten; + + close(fd[0]); + + ret = pidfile_create(NULL, pidfile, &pid_ctx); + + nwritten = write(fd[1], &ret, sizeof(ret)); + assert(nwritten == sizeof(ret)); + + sleep(99); + exit(1); + } + + close(fd[1]); + + nread = read(fd[0], &ret, sizeof(ret)); + assert(nread == sizeof(ret)); + assert(ret == 0); + + ret = stat(pidfile, &st); + assert(ret == 0); + + ret = kill(pid, SIGKILL); + assert(ret == 0); + + pid2 = waitpid(pid, &ret, 0); + assert(pid2 == pid); + + ret = pidfile_create(NULL, pidfile, &pid_ctx); + assert(ret == 0); + assert(pid_ctx != NULL); + + ret = stat(pidfile, &st); + assert(ret == 0); + + TALLOC_FREE(pid_ctx); +} + +int main(int argc, const char **argv) +{ + if (argc != 2) { + fprintf(stderr, "Usage: %s \n", argv[0]); + exit(1); + } + + test1(argv[1]); + test2(argv[1]); + test3(argv[1]); + test4(argv[1]); + + return 0; +} diff --git a/ctdb/wscript b/ctdb/wscript index 5990fc5..04a3766 100755 --- a/ctdb/wscript +++ b/ctdb/wscript @@ -343,7 +343,7 @@ def build(bld): source=bld.SUBDIR('common', '''db_hash.c srvid.c reqid.c pkt_read.c pkt_write.c comm.c - logging.c'''), + logging.c pidfile.c'''), deps='replace talloc tevent tdb tevent-util') bld.SAMBA_SUBSYSTEM('ctdb-protocol', @@ -629,6 +629,7 @@ def build(bld): 'comm_client_test', 'protocol_types_test', 'protocol_client_test', + 'pidfile_test', ] for target in ctdb_unit_tests: -- 2.7.4 From bb90ace3e635471b1c3aa6da9fb3dd777bfd6082 Mon Sep 17 00:00:00 2001 From: Martin Schwenke Date: Thu, 22 Sep 2016 14:35:03 +1000 Subject: [PATCH 2/6] ctdb-daemon: Use PID file abstraction BUG: https://bugzilla.samba.org/show_bug.cgi?id=12287 Signed-off-by: Martin Schwenke Reviewed-by: Amitay Isaacs (cherry picked from commit 5148e02adb7b2ea34da9c826a682c1387773402b) --- ctdb/server/ctdb_daemon.c | 31 +++++++++++-------------------- 1 file changed, 11 insertions(+), 20 deletions(-) diff --git a/ctdb/server/ctdb_daemon.c b/ctdb/server/ctdb_daemon.c index 1fe792c..b69c32b 100644 --- a/ctdb/server/ctdb_daemon.c +++ b/ctdb/server/ctdb_daemon.c @@ -43,6 +43,7 @@ #include "common/system.h" #include "common/common.h" #include "common/logging.h" +#include "common/pidfile.h" struct ctdb_client_pid_list { struct ctdb_client_pid_list *next, *prev; @@ -52,6 +53,7 @@ struct ctdb_client_pid_list { }; const char *ctdbd_pidfile = NULL; +static struct pidfile_context *ctdbd_pidfile_ctx = NULL; static void daemon_incoming_packet(void *, struct ctdb_req_header *); @@ -1127,32 +1129,21 @@ static void ctdb_tevent_trace(enum tevent_trace_point tp, static void ctdb_remove_pidfile(void) { - /* Only the main ctdbd's PID matches the SID */ - if (ctdbd_pidfile != NULL && getsid(0) == getpid()) { - if (unlink(ctdbd_pidfile) == 0) { - DEBUG(DEBUG_NOTICE, ("Removed PID file %s\n", - ctdbd_pidfile)); - } else { - DEBUG(DEBUG_WARNING, ("Failed to Remove PID file %s\n", - ctdbd_pidfile)); - } - } + TALLOC_FREE(ctdbd_pidfile_ctx); } -static void ctdb_create_pidfile(pid_t pid) +static void ctdb_create_pidfile(TALLOC_CTX *mem_ctx) { if (ctdbd_pidfile != NULL) { - FILE *fp; - - fp = fopen(ctdbd_pidfile, "w"); - if (fp == NULL) { - DEBUG(DEBUG_ALERT, - ("Failed to open PID file %s\n", ctdbd_pidfile)); + int ret = pidfile_create(mem_ctx, ctdbd_pidfile, + &ctdbd_pidfile_ctx); + if (ret != 0) { + DEBUG(DEBUG_ERR, + ("Failed to create PID file %s\n", + ctdbd_pidfile)); exit(11); } - fprintf(fp, "%d\n", pid); - fclose(fp); DEBUG(DEBUG_NOTICE, ("Created PID file %s\n", ctdbd_pidfile)); atexit(ctdb_remove_pidfile); } @@ -1243,7 +1234,7 @@ int ctdb_start_daemon(struct ctdb_context *ctdb, bool do_fork) ctdb->ctdbd_pid = getpid(); DEBUG(DEBUG_ERR, ("Starting CTDBD (Version %s) as PID: %u\n", CTDB_VERSION_STRING, ctdb->ctdbd_pid)); - ctdb_create_pidfile(ctdb->ctdbd_pid); + ctdb_create_pidfile(ctdb); /* Make sure we log something when the daemon terminates. * This must be the first exit handler to run (so the last to -- 2.7.4 From 1c4946a8af728818da1dd1cf6c7a0d9eea1de9fd Mon Sep 17 00:00:00 2001 From: Martin Schwenke Date: Thu, 22 Sep 2016 14:43:58 +1000 Subject: [PATCH 3/6] ctdb-daemon: Bind to Unix domain socket after PID file creation No use touching the socket if PID file creation fails. BUG: https://bugzilla.samba.org/show_bug.cgi?id=12287 Signed-off-by: Martin Schwenke Reviewed-by: Amitay Isaacs (cherry picked from commit 1e501c77492d25b760c7b10849460ee6490f39dc) --- ctdb/server/ctdb_daemon.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/ctdb/server/ctdb_daemon.c b/ctdb/server/ctdb_daemon.c index b69c32b..e051984 100644 --- a/ctdb/server/ctdb_daemon.c +++ b/ctdb/server/ctdb_daemon.c @@ -1205,13 +1205,6 @@ int ctdb_start_daemon(struct ctdb_context *ctdb, bool do_fork) int res, ret = -1; struct tevent_fd *fde; - /* create a unix domain stream socket to listen to */ - res = ux_socket_bind(ctdb); - if (res!=0) { - DEBUG(DEBUG_ALERT,("Cannot continue. Exiting!\n")); - exit(10); - } - if (do_fork && fork()) { return 0; } @@ -1236,6 +1229,13 @@ int ctdb_start_daemon(struct ctdb_context *ctdb, bool do_fork) CTDB_VERSION_STRING, ctdb->ctdbd_pid)); ctdb_create_pidfile(ctdb); + /* create a unix domain stream socket to listen to */ + res = ux_socket_bind(ctdb); + if (res!=0) { + DEBUG(DEBUG_ALERT,("Cannot continue. Exiting!\n")); + exit(10); + } + /* Make sure we log something when the daemon terminates. * This must be the first exit handler to run (so the last to * be registered. -- 2.7.4 From 6b2f6552de5feed05bcd10f7dffb3ddeadfe0511 Mon Sep 17 00:00:00 2001 From: Martin Schwenke Date: Thu, 22 Sep 2016 14:46:12 +1000 Subject: [PATCH 4/6] ctdb-daemon: Don't try to reopen TDB files There aren't any open at this stage. BUG: https://bugzilla.samba.org/show_bug.cgi?id=12287 Signed-off-by: Martin Schwenke Reviewed-by: Amitay Isaacs (cherry picked from commit d719a87fe021b0c704fc4b12ddfc0345fe3af146) --- ctdb/server/ctdb_daemon.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/ctdb/server/ctdb_daemon.c b/ctdb/server/ctdb_daemon.c index e051984..26efd20 100644 --- a/ctdb/server/ctdb_daemon.c +++ b/ctdb/server/ctdb_daemon.c @@ -1209,8 +1209,6 @@ int ctdb_start_daemon(struct ctdb_context *ctdb, bool do_fork) return 0; } - tdb_reopen_all(false); - if (do_fork) { if (setsid() == -1) { ctdb_die(ctdb, "Failed to setsid()\n"); -- 2.7.4 From 30932bd419f88f7ef1d492fdc2bbec8d3c83689c Mon Sep 17 00:00:00 2001 From: Martin Schwenke Date: Thu, 22 Sep 2016 14:47:02 +1000 Subject: [PATCH 5/6] ctdb-daemon: Drop attempt to connect to Unix domain socket This was a weak attempt at exclusivity. PID file creation now does that properly. BUG: https://bugzilla.samba.org/show_bug.cgi?id=12287 Signed-off-by: Martin Schwenke Reviewed-by: Amitay Isaacs (cherry picked from commit 8eff9e96037627b1e4adf3ccc8da94ef8f0bad2a) --- ctdb/server/ctdb_daemon.c | 9 --------- 1 file changed, 9 deletions(-) diff --git a/ctdb/server/ctdb_daemon.c b/ctdb/server/ctdb_daemon.c index 26efd20..d205423 100644 --- a/ctdb/server/ctdb_daemon.c +++ b/ctdb/server/ctdb_daemon.c @@ -995,15 +995,6 @@ static int ux_socket_bind(struct ctdb_context *ctdb) addr.sun_family = AF_UNIX; strncpy(addr.sun_path, ctdb->daemon.name, sizeof(addr.sun_path)-1); - /* First check if an old ctdbd might be running */ - if (connect(ctdb->daemon.sd, - (struct sockaddr *)&addr, sizeof(addr)) == 0) { - DEBUG(DEBUG_CRIT, - ("Something is already listening on ctdb socket '%s'\n", - ctdb->daemon.name)); - goto failed; - } - /* Remove any old socket */ unlink(ctdb->daemon.name); -- 2.7.4 From a8b947032d6706b27edd66a9df6829f17798f098 Mon Sep 17 00:00:00 2001 From: Martin Schwenke Date: Thu, 22 Sep 2016 14:52:55 +1000 Subject: [PATCH 6/6] ctdb-daemon: Log when removing stale Unix domain socket BUG: https://bugzilla.samba.org/show_bug.cgi?id=12287 Signed-off-by: Martin Schwenke Reviewed-by: Amitay Isaacs Autobuild-User(master): Amitay Isaacs Autobuild-Date(master): Thu Sep 22 12:28:12 CEST 2016 on sn-devel-144 (cherry picked from commit 0ec01826d32019b06dd10bb9b6ea5232786d5699) --- ctdb/server/ctdb_daemon.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/ctdb/server/ctdb_daemon.c b/ctdb/server/ctdb_daemon.c index d205423..1cc74a0 100644 --- a/ctdb/server/ctdb_daemon.c +++ b/ctdb/server/ctdb_daemon.c @@ -985,6 +985,7 @@ static void ctdb_accept_client(struct tevent_context *ev, static int ux_socket_bind(struct ctdb_context *ctdb) { struct sockaddr_un addr; + int ret; ctdb->daemon.sd = socket(AF_UNIX, SOCK_STREAM, 0); if (ctdb->daemon.sd == -1) { @@ -996,7 +997,15 @@ static int ux_socket_bind(struct ctdb_context *ctdb) strncpy(addr.sun_path, ctdb->daemon.name, sizeof(addr.sun_path)-1); /* Remove any old socket */ - unlink(ctdb->daemon.name); + ret = unlink(ctdb->daemon.name); + if (ret == 0) { + DEBUG(DEBUG_WARNING, + ("Removed stale socket %s\n", ctdb->daemon.name)); + } else if (errno != ENOENT) { + DEBUG(DEBUG_ERR, + ("Failed to remove stale socket %s\n", ctdb->daemon.name)); + return -1; + } set_close_on_exec(ctdb->daemon.sd); set_nonblocking(ctdb->daemon.sd); -- 2.7.4