From a5035d44b9049e26924ee086d691aa550696966b Mon Sep 17 00:00:00 2001 From: nicm Date: Mon, 21 Jul 2014 10:52:48 +0000 Subject: [PATCH] lockf is entirely useless and it was a mistake to change to it, go back to using flock which actually works sensibly. Also always retry the lock to fix a potential race, and add some extra logging. --- usr.bin/tmux/client.c | 50 ++++++++++++++++++++++++++++++++----------- usr.bin/tmux/server.c | 3 ++- 2 files changed, 39 insertions(+), 14 deletions(-) diff --git a/usr.bin/tmux/client.c b/usr.bin/tmux/client.c index ce2d68ed2f9..10d2d20a761 100644 --- a/usr.bin/tmux/client.c +++ b/usr.bin/tmux/client.c @@ -1,4 +1,4 @@ -/* $OpenBSD: client.c,v 1.81 2014/07/13 20:51:08 krw Exp $ */ +/* $OpenBSD: client.c,v 1.82 2014/07/21 10:52:48 nicm Exp $ */ /* * Copyright (c) 2007 Nicholas Marriott @@ -77,13 +77,18 @@ client_get_lock(char *lockfile) if ((lockfd = open(lockfile, O_WRONLY|O_CREAT, 0600)) == -1) fatal("open failed"); + log_debug("lock file is %s", lockfile); - if (lockf(lockfd, F_TLOCK, 0) == -1 && errno == EAGAIN) { - while (lockf(lockfd, F_LOCK, 0) == -1 && errno == EINTR) + if (flock(lockfd, LOCK_EX|LOCK_NB) == -1) { + log_debug("flock failed: %s", strerror(errno)); + if (errno != EAGAIN) + return (lockfd); + while (flock(lockfd, LOCK_EX) == -1 && errno == EINTR) /* nothing */; close(lockfd); return (-1); } + log_debug("flock succeeded"); return (lockfd); } @@ -94,8 +99,8 @@ client_connect(char *path, int start_server) { struct sockaddr_un sa; size_t size; - int fd, lockfd; - char *lockfile; + int fd, lockfd = -1, locked = 0; + char *lockfile = NULL; memset(&sa, 0, sizeof sa); sa.sun_family = AF_UNIX; @@ -104,29 +109,48 @@ client_connect(char *path, int start_server) errno = ENAMETOOLONG; return (-1); } + log_debug("socket is %s", path); retry: if ((fd = socket(AF_UNIX, SOCK_STREAM, 0)) == -1) fatal("socket failed"); + log_debug("trying connect"); if (connect(fd, (struct sockaddr *) &sa, SUN_LEN(&sa)) == -1) { + log_debug("connect failed: %s", strerror(errno)); if (errno != ECONNREFUSED && errno != ENOENT) goto failed; if (!start_server) goto failed; close(fd); - xasprintf(&lockfile, "%s.lock", path); - if ((lockfd = client_get_lock(lockfile)) == -1) { - free(lockfile); + if (!locked) { + xasprintf(&lockfile, "%s.lock", path); + if ((lockfd = client_get_lock(lockfile)) == -1) { + log_debug("didn't get lock"); + free(lockfile); + goto retry; + } + log_debug("got lock"); + + /* + * Always retry at least once, even if we got the lock, + * because another client could have taken the lock, + * started the server and released the lock between our + * connect() and flock(). + */ + locked = 1; goto retry; } + if (unlink(path) != 0 && errno != ENOENT) { free(lockfile); close(lockfd); return (-1); } fd = server_start(lockfd, lockfile); + } + if (locked) { free(lockfile); close(lockfd); } @@ -233,7 +257,11 @@ client_main(int argc, char **argv, int flags) return (1); } - /* Initialise the client socket and start the server. */ + /* Set process title, log and signals now this is the client. */ + setproctitle("client (%s)", socket_path); + logfile("client"); + + /* Initialize the client socket and start the server. */ fd = client_connect(socket_path, cmdflags & CMD_STARTSERVER); if (fd == -1) { fprintf(stderr, "failed to connect to server: %s\n", @@ -241,10 +269,6 @@ client_main(int argc, char **argv, int flags) return (1); } - /* Set process title, log and signals now this is the client. */ - setproctitle("client (%s)", socket_path); - logfile("client"); - /* Create imsg. */ imsg_init(&client_ibuf, fd); event_set(&client_event, fd, EV_READ, client_callback, shell_cmd); diff --git a/usr.bin/tmux/server.c b/usr.bin/tmux/server.c index f3706aed213..3289dc2196b 100644 --- a/usr.bin/tmux/server.c +++ b/usr.bin/tmux/server.c @@ -1,4 +1,4 @@ -/* $OpenBSD: server.c,v 1.114 2014/05/14 06:21:19 nicm Exp $ */ +/* $OpenBSD: server.c,v 1.115 2014/07/21 10:52:48 nicm Exp $ */ /* * Copyright (c) 2007 Nicholas Marriott @@ -111,6 +111,7 @@ server_start(int lockfd, char *lockfile) /* The first client is special and gets a socketpair; create it. */ if (socketpair(AF_UNIX, SOCK_STREAM, PF_UNSPEC, pair) != 0) fatal("socketpair failed"); + log_debug("starting server"); switch (fork()) { case -1: -- 2.20.1