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.
This commit is contained in:
nicm 2014-07-21 10:52:48 +00:00
parent 2056a9ef9e
commit 8e4ae12b4d
2 changed files with 37 additions and 12 deletions

View File

@ -77,13 +77,18 @@ client_get_lock(char *lockfile)
if ((lockfd = open(lockfile, O_WRONLY|O_CREAT, 0600)) == -1) if ((lockfd = open(lockfile, O_WRONLY|O_CREAT, 0600)) == -1)
fatal("open failed"); fatal("open failed");
log_debug("lock file is %s", lockfile);
if (lockf(lockfd, F_TLOCK, 0) == -1 && errno == EAGAIN) { if (flock(lockfd, LOCK_EX|LOCK_NB) == -1) {
while (lockf(lockfd, F_LOCK, 0) == -1 && errno == EINTR) log_debug("flock failed: %s", strerror(errno));
if (errno != EAGAIN)
return (lockfd);
while (flock(lockfd, LOCK_EX) == -1 && errno == EINTR)
/* nothing */; /* nothing */;
close(lockfd); close(lockfd);
return (-1); return (-1);
} }
log_debug("flock succeeded");
return (lockfd); return (lockfd);
} }
@ -94,8 +99,8 @@ client_connect(char *path, int start_server)
{ {
struct sockaddr_un sa; struct sockaddr_un sa;
size_t size; size_t size;
int fd, lockfd; int fd, lockfd = -1, locked = 0;
char *lockfile; char *lockfile = NULL;
memset(&sa, 0, sizeof sa); memset(&sa, 0, sizeof sa);
sa.sun_family = AF_UNIX; sa.sun_family = AF_UNIX;
@ -104,29 +109,48 @@ client_connect(char *path, int start_server)
errno = ENAMETOOLONG; errno = ENAMETOOLONG;
return (-1); return (-1);
} }
log_debug("socket is %s", path);
retry: retry:
if ((fd = socket(AF_UNIX, SOCK_STREAM, 0)) == -1) if ((fd = socket(AF_UNIX, SOCK_STREAM, 0)) == -1)
fatal("socket failed"); fatal("socket failed");
log_debug("trying connect");
if (connect(fd, (struct sockaddr *) &sa, SUN_LEN(&sa)) == -1) { if (connect(fd, (struct sockaddr *) &sa, SUN_LEN(&sa)) == -1) {
log_debug("connect failed: %s", strerror(errno));
if (errno != ECONNREFUSED && errno != ENOENT) if (errno != ECONNREFUSED && errno != ENOENT)
goto failed; goto failed;
if (!start_server) if (!start_server)
goto failed; goto failed;
close(fd); close(fd);
xasprintf(&lockfile, "%s.lock", path); if (!locked) {
if ((lockfd = client_get_lock(lockfile)) == -1) { xasprintf(&lockfile, "%s.lock", path);
free(lockfile); 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; goto retry;
} }
if (unlink(path) != 0 && errno != ENOENT) { if (unlink(path) != 0 && errno != ENOENT) {
free(lockfile); free(lockfile);
close(lockfd); close(lockfd);
return (-1); return (-1);
} }
fd = server_start(lockfd, lockfile); fd = server_start(lockfd, lockfile);
}
if (locked) {
free(lockfile); free(lockfile);
close(lockfd); close(lockfd);
} }
@ -233,7 +257,11 @@ client_main(int argc, char **argv, int flags)
return (1); 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); fd = client_connect(socket_path, cmdflags & CMD_STARTSERVER);
if (fd == -1) { if (fd == -1) {
fprintf(stderr, "failed to connect to server: %s\n", fprintf(stderr, "failed to connect to server: %s\n",
@ -241,10 +269,6 @@ client_main(int argc, char **argv, int flags)
return (1); return (1);
} }
/* Set process title, log and signals now this is the client. */
setproctitle("client (%s)", socket_path);
logfile("client");
/* Create imsg. */ /* Create imsg. */
imsg_init(&client_ibuf, fd); imsg_init(&client_ibuf, fd);
event_set(&client_event, fd, EV_READ, client_callback, shell_cmd); event_set(&client_event, fd, EV_READ, client_callback, shell_cmd);

View File

@ -111,6 +111,7 @@ server_start(int lockfd, char *lockfile)
/* The first client is special and gets a socketpair; create it. */ /* The first client is special and gets a socketpair; create it. */
if (socketpair(AF_UNIX, SOCK_STREAM, PF_UNSPEC, pair) != 0) if (socketpair(AF_UNIX, SOCK_STREAM, PF_UNSPEC, pair) != 0)
fatal("socketpair failed"); fatal("socketpair failed");
log_debug("starting server");
switch (fork()) { switch (fork()) {
case -1: case -1: