From 12acb70c4e33307d0bbe21236a8b211e2e33da58 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/pidfile_test.c | 241 +++++++++++++++++++++++++++++++++++ ctdb/wscript | 4 +- 5 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/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 1e80e01..5675849 100644 --- a/ctdb/wscript +++ b/ctdb/wscript @@ -346,7 +346,8 @@ def build(bld): source=bld.SUBDIR('common', '''db_hash.c srvid.c reqid.c pkt_read.c pkt_write.c comm.c - logging.c rb_tree.c tunable.c'''), + logging.c rb_tree.c tunable.c + pidfile.c'''), deps='''samba-util tevent-util replace talloc tevent tdb''') @@ -657,6 +658,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 cd5e43b0b26a2b7ca1d2c3fb57af3ea48d2a1909 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 47e49df..d3ea263 100644 --- a/ctdb/server/ctdb_daemon.c +++ b/ctdb/server/ctdb_daemon.c @@ -44,6 +44,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; @@ -53,6 +54,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 *); @@ -1149,32 +1151,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); } @@ -1265,7 +1256,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 12f99bd5ae5ee00bb984e21724d9a6d26ab5ef7e 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 d3ea263..1fb8a3f 100644 --- a/ctdb/server/ctdb_daemon.c +++ b/ctdb/server/ctdb_daemon.c @@ -1227,13 +1227,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; } @@ -1258,6 +1251,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 24fc9037518f191c6a7e8aa2737a42353bdc16f2 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 1fb8a3f..0202fc1 100644 --- a/ctdb/server/ctdb_daemon.c +++ b/ctdb/server/ctdb_daemon.c @@ -1231,8 +1231,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 56d58ed66362db17c65af19eb4348c705d8b5783 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 0202fc1..f01193a 100644 --- a/ctdb/server/ctdb_daemon.c +++ b/ctdb/server/ctdb_daemon.c @@ -1007,15 +1007,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 098334a1d8b5ce5a2c420b7bfd0e55156db21567 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 | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/ctdb/server/ctdb_daemon.c b/ctdb/server/ctdb_daemon.c index f01193a..8cd5bba 100644 --- a/ctdb/server/ctdb_daemon.c +++ b/ctdb/server/ctdb_daemon.c @@ -1008,7 +1008,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); -- 2.7.4