commit 1d6739509f8059b45ef1c1eb4f06d4596fc46984 from: Omar Polo date: Thu Mar 03 13:50:12 2022 UTC rework how the daemon is automatically spawned The previous way introduce possible races if multiple `amused' instances try to start the daemon. The new approach is heavily lifted from how tmux does it, but with some minor differences. If the initial connect fails we try to grab a lock to safely execute the daemon only one time. In fact we try to connect one more time even when we hold the lock because another instance can grab the lock, start the daemon and release it between the failure of connect and the first flock. It also changes slightly how the program behaves and how the -d option works. Now running `amused' without arguments is a synonym for `amused status' and the -d option only works if no arguments were given and if the daemon wasn't running. commit - 494d8ca239824a4809e171c484f2a9518a0dc6a1 commit + 1d6739509f8059b45ef1c1eb4f06d4596fc46984 blob - 74d82695ab2684ecccc3055fe046818b830b2728 blob + 4eae07ace3e8e07427b651f85dce5b6ad032aad4 --- amused.1 +++ amused.1 @@ -22,25 +22,25 @@ .Nm .Op Fl dv .Op Fl s Ar socket +.Oo .Ar command .Op Ar argument ... +.Oc .Sh DESCRIPTION .Nm is a music player daemon and command-line utility that plays music in the background. The server is automatically started when the user interacts with -it, but it can be started manually by running -.Nm -without any arguments. +it. .Pp The following options are available: .Bl -tag -width 12m .It Fl d -Do not daemonize. -If this option is specified, +Do not daemonize if starting the daemon: .Nm will run in the foreground and log to standard error. -It's ignored if any commands are given on the command line. +It's ignored if any commands are given on the command line or if the +server was already running. .It Fl v Produce more verbose output. .It Fl s Ar socket @@ -49,6 +49,11 @@ Use instead of the default .Pa /tmp/amused-$UID to communicate with the daemon. +.It Ar command Op Ar argument ... +Specify the command to run. +If no commands are specified, the +.Ic status +command is assumed. .El .Pp The following commands are available: blob - 0356b1c0e2de75a68ee3e88790c20939b4e8e37a blob + 300ba43879584007e51ad4ee42fe73e3f6b8ab3e --- amused.c +++ amused.c @@ -165,10 +165,13 @@ main_dispatch_player(int sig, short event, void *d) static pid_t start_child(enum amused_process proc, int fd) { - const char *argv[6]; + const char *argv[7]; int argc = 0; pid_t pid; + if (fd == -1 && debug) + goto exec; + switch (pid = fork()) { case -1: fatal("cannot fork"); @@ -185,12 +188,14 @@ start_child(enum amused_process proc, int fd) } else if (fcntl(F_SETFD, 0) == -1) fatal("cannot setup imsg fd"); +exec: argv[argc++] = argv0; switch (proc) { case PROC_MAIN: argv[argc++] = "-s"; argv[argc++] = csock; + argv[argc++] = "-Tm"; break; case PROC_PLAYER: argv[argc++] = "-Tp"; @@ -263,7 +268,7 @@ amused_main(void) int main(int argc, char **argv) { - int ch, proc = PROC_MAIN; + int ch, proc = -1; log_init(1, LOG_DAEMON); /* Log to stderr until daemonized */ log_setverbose(1); @@ -283,6 +288,9 @@ main(int argc, char **argv) break; case 'T': switch (*optarg) { + case 'm': + proc = PROC_MAIN; + break; case 'p': proc = PROC_PLAYER; break; @@ -300,23 +308,23 @@ main(int argc, char **argv) argv += optind; argc -= optind; + if (proc == PROC_MAIN) + amused_main(); if (proc == PROC_PLAYER) exit(player(debug, verbose)); if (csock == NULL) xasprintf(&csock, "/tmp/amused-%d", getuid()); - if (argc == 0) - amused_main(); - else - ctl(argc, argv); - return 0; + if (argc > 0) + debug = 0; + + ctl(argc, argv); } void spawn_daemon(void) { - debug = 0; start_child(PROC_MAIN, -1); } blob - d59f7739bf42943ea1e737d7260b70a88477c0ab blob + 66425e68fc874dbedd997780e157dd44f6757cf6 --- ctl.c +++ ctl.c @@ -22,6 +22,7 @@ #include #include +#include #include #include #include @@ -85,16 +86,20 @@ parse(int argc, char **argv) { struct ctl_command *ctl = NULL; struct parse_result res; + const char *argv0; int i, status; memset(&res, 0, sizeof(res)); + if ((argv0 = argv[0]) == NULL) + argv0 = "status"; + for (i = 0; ctl_commands[i].name != NULL; ++i) { - if (strncmp(ctl_commands[i].name, argv[0], strlen(argv[0])) + if (strncmp(ctl_commands[i].name, argv0, strlen(argv0)) == 0) { if (ctl != NULL) { fprintf(stderr, - "ambiguous argument: %s\n", argv[0]); + "ambiguous argument: %s\n", argv0); usage(); } ctl = &ctl_commands[i]; @@ -650,49 +655,115 @@ ctl_repeat(struct parse_result *res, int argc, char ** } static int -sockconn(void) +ctl_get_lock(const char *lockfile) { - struct sockaddr_un sun; - int sock, saved_errno; + int lockfd; - if ((sock = socket(AF_UNIX, SOCK_STREAM, 0)) == -1) - fatal("socket"); + if ((lockfd = open(lockfile, O_WRONLY|O_CREAT, 0600)) == -1) { + log_debug("open failed: %s", strerror(errno)); + return -1; + } - memset(&sun, 0, sizeof(sun)); - sun.sun_family = AF_UNIX; - strlcpy(sun.sun_path, csock, sizeof(sun.sun_path)); - if (connect(sock, (struct sockaddr *)&sun, sizeof(sun)) == -1) { - saved_errno = errno; - close(sock); - errno = saved_errno; + if (flock(lockfd, LOCK_EX|LOCK_NB) == -1) { + log_debug("flock failed: %s", strerror(errno)); + if (errno != EAGAIN) + return -1; + while (flock(lockfd, LOCK_EX) == -1 && errno == EINTR) + /* nop */; + close(lockfd); + return -2; + } + log_debug("flock succeeded"); + + return lockfd; +} + +static int +ctl_connect(void) +{ + struct timespec ts = { 0, 50000000 }; /* 0.05 seconds */ + struct sockaddr_un sa; + size_t size; + int fd, lockfd = -1, locked = 0, spawned = 0; + char *lockfile = NULL; + + memset(&sa, 0, sizeof(sa)); + sa.sun_family = AF_UNIX; + size = strlcpy(sa.sun_path, csock, sizeof(sa.sun_path)); + if (size >= sizeof(sa.sun_path)) { + errno = ENAMETOOLONG; return -1; } - return sock; +retry: + if ((fd = socket(AF_UNIX, SOCK_STREAM, 0)) == -1) + return -1; + + if (connect(fd, (struct sockaddr *)&sa, sizeof(sa)) == -1) { + log_debug("connection failed: %s", strerror(errno)); + if (errno != ECONNREFUSED && errno != ENOENT) + goto failed; + close(fd); + + if (!locked) { + xasprintf(&lockfile, "%s.lock", csock); + if ((lockfd = ctl_get_lock(lockfile)) < 0) { + log_debug("didn't get the lock (%d)", lockfd); + + free(lockfile); + lockfile = NULL; + + if (lockfd == -1) + goto retry; + } + + /* + * 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 (!spawned) { + log_debug("spawning the daemon"); + spawn_daemon(); + spawned = 1; + } + + nanosleep(&ts, NULL); + goto retry; + } + + if (locked && lockfd >= 0) { + unlink(lockfile); + free(lockfile); + close(lockfd); + } + return fd; + +failed: + if (locked) { + free(lockfile); + close(lockfd); + } + close(fd); + return -1; } __dead void ctl(int argc, char **argv) { - int ctl_sock, i = 0; + int ctl_sock; log_init(1, LOG_DAEMON); log_setverbose(verbose); - do { - struct timespec ts = { 0, 50000000 }; /* 0.05 seconds */ + if ((ctl_sock = ctl_connect()) == -1) + fatal("can't connect"); - if ((ctl_sock = sockconn()) != -1) - break; - if (errno != ENOENT && errno != ECONNREFUSED) - fatal("connect %s", csock); - - if (i == 0) - spawn_daemon(); - - nanosleep(&ts, NULL); - } while (++i < 20); - if (ctl_sock == -1) fatalx("failed to connect to the daemon");