[SCM] Samba Shared Repository - branch master updated
Martin Schwenke
martins at samba.org
Wed Aug 2 05:29:02 UTC 2017
The branch, master has been updated
via edf77a1 ctdb-common: Reimplement pidfile_context_create() using pidfile_path_create()
via 09b0398 util: Reimplement pidfile_create() using pidfile_path_create()
via 411b7c8 util: New functions pidfile_path_create(), pidfile_fd_close()
via 59ebb29 ctdb-common: Rename pidfile_create() -> pidfile_context_create()
via 4dbfc16 util: Add pidfile.* to samba-util-core
via d665b74 util: Clean up includes
via 8731f1c util: pidfile_pid() should not unlink PID file
from a70ab5f winbindd: Simplify two debug msgs
https://git.samba.org/?p=samba.git;a=shortlog;h=master
- Log -----------------------------------------------------------------
commit edf77a112f44a3eda43c3d6cdb2985faf1335d35
Author: Martin Schwenke <martin at meltin.net>
Date: Mon Jul 31 15:26:36 2017 +1000
ctdb-common: Reimplement pidfile_context_create() using pidfile_path_create()
Signed-off-by: Martin Schwenke <martin at meltin.net>
Reviewed-by: Volker Lendecke <vl at samba.org>
Autobuild-User(master): Martin Schwenke <martins at samba.org>
Autobuild-Date(master): Wed Aug 2 07:28:44 CEST 2017 on sn-devel-144
commit 09b03988650b68897c02a521d570737578c72d85
Author: Martin Schwenke <martin at meltin.net>
Date: Mon Jul 31 15:20:19 2017 +1000
util: Reimplement pidfile_create() using pidfile_path_create()
Signed-off-by: Martin Schwenke <martin at meltin.net>
Reviewed-by: Volker Lendecke <vl at samba.org>
commit 411b7c87536a8639954bf1eb6876209abfb4e848
Author: Martin Schwenke <martin at meltin.net>
Date: Mon Jul 31 15:11:33 2017 +1000
util: New functions pidfile_path_create(), pidfile_fd_close()
Uses the core of CTDB's create_pidfile_context() for
pidfile_path_create(). pidfile_fd_close() is a subset of CTDB's
pidfile_context_destructor().
Signed-off-by: Martin Schwenke <martin at meltin.net>
Reviewed-by: Volker Lendecke <vl at samba.org>
commit 59ebb29e6a597586338c610b22927e5bd8a7628c
Author: Martin Schwenke <martin at meltin.net>
Date: Mon Jul 31 15:16:45 2017 +1000
ctdb-common: Rename pidfile_create() -> pidfile_context_create()
Signed-off-by: Martin Schwenke <martin at meltin.net>
Reviewed-by: Volker Lendecke <vl at samba.org>
commit 4dbfc16bdd6253f887fd34bef8db5464c363e0e8
Author: Martin Schwenke <martin at meltin.net>
Date: Mon Jul 31 14:48:47 2017 +1000
util: Add pidfile.* to samba-util-core
Signed-off-by: Martin Schwenke <martin at meltin.net>
Reviewed-by: Volker Lendecke <vl at samba.org>
commit d665b74dbb4986c58fd887d7425d054c0533c924
Author: Martin Schwenke <martin at meltin.net>
Date: Mon Jul 31 14:47:01 2017 +1000
util: Clean up includes
Signed-off-by: Martin Schwenke <martin at meltin.net>
Reviewed-by: Volker Lendecke <vl at samba.org>
commit 8731f1cfab5e8cd2550237b589b5767a93935381
Author: Martin Schwenke <martin at meltin.net>
Date: Mon Jul 31 11:37:21 2017 +1000
util: pidfile_pid() should not unlink PID file
This causes a race. If 2 callers to pidfile_create() both a find a
stale PID file using pidfile_pid(). The 1st may then return to
pidfile_create() and create a new PID file, which can then be unlinked
by the 2nd caller.
Consequently, PID file creation can not depend on creating the file,
so drop O_EXCL from the call to open().
Signed-off-by: Martin Schwenke <martin at meltin.net>
Reviewed-by: Volker Lendecke <vl at samba.org>
-----------------------------------------------------------------------
Summary of changes:
ctdb/common/pidfile.c | 80 +++---------------------
ctdb/common/pidfile.h | 4 +-
ctdb/common/sock_daemon.c | 2 +-
ctdb/server/ctdb_daemon.c | 4 +-
ctdb/tests/src/pidfile_test.c | 14 ++---
lib/util/pidfile.c | 138 +++++++++++++++++++++++++++++++++---------
lib/util/pidfile.h | 3 +
lib/util/wscript_build | 4 +-
8 files changed, 134 insertions(+), 115 deletions(-)
Changeset truncated at 500 lines:
diff --git a/ctdb/common/pidfile.c b/ctdb/common/pidfile.c
index 51c0c25..e78542d 100644
--- a/ctdb/common/pidfile.c
+++ b/ctdb/common/pidfile.c
@@ -23,6 +23,7 @@
#include <talloc.h>
#include "lib/util/blocking.h"
+#include "lib/util/pidfile.h"
#include "common/pidfile.h"
@@ -34,15 +35,11 @@ struct pidfile_context {
static int pidfile_context_destructor(struct pidfile_context *pid_ctx);
-int pidfile_create(TALLOC_CTX *mem_ctx, const char *pidfile,
- struct pidfile_context **result)
+int pidfile_context_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) {
@@ -57,67 +54,18 @@ int pidfile_create(TALLOC_CTX *mem_ctx, const char *pidfile,
pid_ctx->pid = getpid();
- fd = open(pidfile, O_CREAT|O_WRONLY|O_NONBLOCK, 0644);
- if (fd == -1) {
- ret = errno;
- goto fail;
- }
-
- if (! set_close_on_exec(fd)) {
- close(fd);
- ret = EIO;
- 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));
-
+ ret = pidfile_path_create(pid_ctx->pidfile, &fd);
if (ret != 0) {
- ret = errno;
- goto fail;
+ return ret;
}
- 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;
- }
+ pid_ctx->fd = fd;
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;
@@ -125,27 +73,13 @@ fail:
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));
+ pidfile_fd_close(pid_ctx->fd);
return 0;
}
diff --git a/ctdb/common/pidfile.h b/ctdb/common/pidfile.h
index 1450134..bc4e3a7 100644
--- a/ctdb/common/pidfile.h
+++ b/ctdb/common/pidfile.h
@@ -45,7 +45,7 @@ struct pidfile_context;
*
* Freeing the pidfile_context, will delete the pidfile.
*/
-int pidfile_create(TALLOC_CTX *mem_ctx, const char *pidfile,
- struct pidfile_context **result);
+int pidfile_context_create(TALLOC_CTX *mem_ctx, const char *pidfile,
+ struct pidfile_context **result);
#endif /* __CTDB_PIDFILE_H__ */
diff --git a/ctdb/common/sock_daemon.c b/ctdb/common/sock_daemon.c
index b53b4d8..a6d4301 100644
--- a/ctdb/common/sock_daemon.c
+++ b/ctdb/common/sock_daemon.c
@@ -477,7 +477,7 @@ int sock_daemon_setup(TALLOC_CTX *mem_ctx, const char *daemon_name,
}
if (pidfile != NULL) {
- ret = pidfile_create(sockd, pidfile, &sockd->pid_ctx);
+ ret = pidfile_context_create(sockd, pidfile, &sockd->pid_ctx);
if (ret != 0) {
talloc_free(sockd);
return EEXIST;
diff --git a/ctdb/server/ctdb_daemon.c b/ctdb/server/ctdb_daemon.c
index b5cee61..1030406 100644
--- a/ctdb/server/ctdb_daemon.c
+++ b/ctdb/server/ctdb_daemon.c
@@ -1158,8 +1158,8 @@ static void ctdb_remove_pidfile(void)
static void ctdb_create_pidfile(TALLOC_CTX *mem_ctx)
{
if (ctdbd_pidfile != NULL) {
- int ret = pidfile_create(mem_ctx, ctdbd_pidfile,
- &ctdbd_pidfile_ctx);
+ int ret = pidfile_context_create(mem_ctx, ctdbd_pidfile,
+ &ctdbd_pidfile_ctx);
if (ret != 0) {
DEBUG(DEBUG_ERR,
("Failed to create PID file %s\n",
diff --git a/ctdb/tests/src/pidfile_test.c b/ctdb/tests/src/pidfile_test.c
index 00b64ca..592fc2b 100644
--- a/ctdb/tests/src/pidfile_test.c
+++ b/ctdb/tests/src/pidfile_test.c
@@ -34,7 +34,7 @@ static void test1(const char *pidfile)
FILE *fp;
pid_t pid;
- ret = pidfile_create(NULL, pidfile, &pid_ctx);
+ ret = pidfile_context_create(NULL, pidfile, &pid_ctx);
assert(ret == 0);
assert(pid_ctx != NULL);
@@ -77,7 +77,7 @@ static void test2(const char *pidfile)
close(fd[0]);
- ret = pidfile_create(NULL, pidfile, &pid_ctx);
+ ret = pidfile_context_create(NULL, pidfile, &pid_ctx);
assert(ret == 0);
assert(pid_ctx != NULL);
@@ -107,14 +107,14 @@ static void test2(const char *pidfile)
assert(pid == pid2);
fclose(fp);
- ret = pidfile_create(NULL, pidfile, &pid_ctx);
+ ret = pidfile_context_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);
+ ret = pidfile_context_create(NULL, pidfile, &pid_ctx);
assert(ret == 0);
assert(pid_ctx != NULL);
@@ -134,7 +134,7 @@ static void test3(const char *pidfile)
size_t nread;
struct stat st;
- ret = pidfile_create(NULL, pidfile, &pid_ctx);
+ ret = pidfile_context_create(NULL, pidfile, &pid_ctx);
assert(ret == 0);
assert(pid_ctx != NULL);
@@ -192,7 +192,7 @@ static void test4(const char *pidfile)
close(fd[0]);
- ret = pidfile_create(NULL, pidfile, &pid_ctx);
+ ret = pidfile_context_create(NULL, pidfile, &pid_ctx);
nwritten = write(fd[1], &ret, sizeof(ret));
assert(nwritten == sizeof(ret));
@@ -216,7 +216,7 @@ static void test4(const char *pidfile)
pid2 = waitpid(pid, &ret, 0);
assert(pid2 == pid);
- ret = pidfile_create(NULL, pidfile, &pid_ctx);
+ ret = pidfile_context_create(NULL, pidfile, &pid_ctx);
assert(ret == 0);
assert(pid_ctx != NULL);
diff --git a/lib/util/pidfile.c b/lib/util/pidfile.c
index 5590780..fdf9afb 100644
--- a/lib/util/pidfile.c
+++ b/lib/util/pidfile.c
@@ -1,9 +1,8 @@
-/* this code is broken - there is a race condition with the unlink (tridge) */
-
/*
Unix SMB/CIFS implementation.
pidfile handling
Copyright (C) Andrew Tridgell 1998
+ Copyright (C) Amitay Isaccs 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
@@ -19,8 +18,13 @@
along with this program. If not, see <http://www.gnu.org/licenses/>.
*/
-#include "includes.h"
+#include "replace.h"
#include "system/filesys.h"
+
+#include "lib/util/blocking.h"
+#include "lib/util/debug.h"
+#include "lib/util/samba_util.h" /* For process_exists_by_pid() */
+
#include "lib/util/pidfile.h"
/**
@@ -28,6 +32,104 @@
* @brief Pid file handling
*/
+int pidfile_path_create(const char *path, int *outfd)
+{
+ struct flock lck;
+ char tmp[64] = { 0 };
+ pid_t pid;
+ int fd, ret = 0;
+ int len;
+ ssize_t nwritten;
+
+ pid = getpid();
+
+ fd = open(path, O_CREAT|O_WRONLY|O_NONBLOCK, 0644);
+ if (fd == -1) {
+ return errno;
+ }
+
+ if (! set_close_on_exec(fd)) {
+ close(fd);
+ return EIO;
+ }
+
+ 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;
+ close(fd);
+ return ret;
+ }
+
+ /*
+ * PID file is locked by us so from here on we should unlink
+ * on failure
+ */
+
+ 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);
+ if (len < 0) {
+ ret = errno;
+ goto fail_unlink;
+ }
+ if (len >= sizeof(tmp)) {
+ ret = ENOSPC;
+ goto fail_unlink;
+ }
+
+ do {
+ nwritten = write(fd, tmp, len);
+ } while ((nwritten == -1) && (errno == EINTR));
+
+ if ((nwritten == -1) || (nwritten != len)) {
+ ret = EIO;
+ goto fail_unlink;
+ }
+
+ if (outfd != NULL) {
+ *outfd = fd;
+ }
+ return 0;
+
+fail_unlink:
+ unlink(path);
+ close(fd);
+ return ret;
+}
+
+void pidfile_fd_close(int fd)
+{
+ struct flock lck = {
+ .l_type = F_UNLCK,
+ .l_whence = SEEK_SET,
+ };
+ int ret;
+
+ do {
+ ret = fcntl(fd, F_SETLK, &lck);
+ } while ((ret == -1) && (errno == EINTR));
+
+ do {
+ ret = close(fd);
+ } while ((ret == -1) && (errno == EINTR));
+}
+
+
/**
* return the pid in a pidfile. return 0 if the process (or pidfile)
* does not exist
@@ -79,9 +181,6 @@ pid_t pidfile_pid(const char *piddir, const char *name)
noproc:
close(fd);
- DEBUG(10, ("Deleting %s, since %d is not a Samba process.\n", pidFile,
- (int)ret));
- unlink(pidFile);
return 0;
}
@@ -92,9 +191,8 @@ void pidfile_create(const char *piddir, const char *name)
{
size_t len = strlen(piddir) + strlen(name) + 6;
char pidFile[len];
- int fd;
- char buf[20];
pid_t pid;
+ int ret;
snprintf(pidFile, sizeof(pidFile), "%s/%s.pid", piddir, name);
@@ -105,26 +203,10 @@ void pidfile_create(const char *piddir, const char *name)
exit(1);
}
- fd = open(pidFile, O_NONBLOCK | O_CREAT | O_WRONLY | O_EXCL, 0644);
- if (fd == -1) {
- DEBUG(0,("ERROR: can't open %s: Error was %s\n", pidFile,
- strerror(errno)));
- exit(1);
- }
-
- smb_set_close_on_exec(fd);
-
- if (fcntl_lock(fd,F_SETLK,0,1,F_WRLCK)==false) {
- DEBUG(0,("ERROR: %s : fcntl lock of file %s failed. Error was %s\n",
- name, pidFile, strerror(errno)));
- exit(1);
- }
-
- memset(buf, 0, sizeof(buf));
- slprintf(buf, sizeof(buf) - 1, "%u\n", (unsigned int) getpid());
- if (write(fd, buf, strlen(buf)) != (ssize_t)strlen(buf)) {
- DEBUG(0,("ERROR: can't write to file %s: %s\n",
- pidFile, strerror(errno)));
+ ret = pidfile_path_create(pidFile, NULL);
+ if (ret != 0) {
+ DBG_ERR("ERROR: Failed to create PID file %s (%s)\n",
+ pidFile, strerror(ret));
exit(1);
}
diff --git a/lib/util/pidfile.h b/lib/util/pidfile.h
index d504f52..d51161a 100644
--- a/lib/util/pidfile.h
+++ b/lib/util/pidfile.h
@@ -20,6 +20,9 @@
#ifndef _SAMBA_PIDFILE_H_
#define _SAMBA_PIDFILE_H_
+int pidfile_path_create(const char *path, int *outfd);
+void pidfile_fd_close(int fd);
+
pid_t pidfile_pid(const char *piddir, const char *name);
void pidfile_create(const char *piddir, const char *program_name);
void pidfile_unlink(const char *piddir, const char *program_name);
diff --git a/lib/util/wscript_build b/lib/util/wscript_build
index 1d2f2ba..989db36 100644
--- a/lib/util/wscript_build
+++ b/lib/util/wscript_build
@@ -71,7 +71,7 @@ bld.SAMBA_SUBSYSTEM('samba-util-core',
source='''data_blob.c util_file.c time.c
signal.c util.c idtree.c fault.c
substitute.c util_process.c util_strlist.c
- strv_util.c bitmap.c select.c''',
+ strv_util.c bitmap.c select.c pidfile.c''',
deps='''time-basic samba-debug socket-blocking talloc
tevent execinfo pthread strv''',
local_include=False)
@@ -120,7 +120,7 @@ else:
util_strlist_v3.c util_paths.c
idtree_random.c base64.c
util_str.c util_str_common.c ms_fnmatch.c
- server_id.c dprintf.c pidfile.c
+ server_id.c dprintf.c
tevent_debug.c memcache.c unix_match.c tfork.c''',
deps='samba-util-core DYNCONFIG close-low-fd tini tiniparser genrand',
public_deps='talloc tevent execinfo pthread LIBCRYPTO charset util_setid systemd systemd-daemon',
--
Samba Shared Repository
More information about the samba-cvs
mailing list