Also take flock locks

There are two kinds of locks, so for interoperability, everyone has to
always take both kinds, to avoid the possibility of one client using one
kind and another client using another kind & missing each other.  :(

I wonder what order we're supposed to take them in?  If we don't all
agree, we get deadlocks!  Here, I take them in alphabetical order
(fcntl then flock), in the hope of this being a legible Schelling point.

The test harness does exercise the "take locks in the 'wrong' order" case.
It probably deadlocks with some tiny probability on heavily-loaded
systems.  Sorry.  If this hurts you, feel free to remove it.

diff --git a/tl-append-test.c b/tl-append-test.c
index 04c679e1b2d2e3b482a86c760b54a109650574ca..e712c6f9b9fd0cb8108cfe882e27562ec2f3f41e 100644 (file)
--- a/tl-append-test.c
+++ b/tl-append-test.c
@@ -255,7 +255,7 @@ static void write_and_read_two_lines() {
 static void write_to_locked_log(char *lock_types[]) {
   remove_logfile();
   ex_t e1 = write_to_tl_append("begin\n");
-  FILE *f = fopen(FILENAME, "a");
+  FILE *f = fopen(FILENAME, "ae");
   if (f == NULL)
     die_err("Couldn't open file for locking");
   for (int i = 0; lock_types[i]; i++)
@@ -313,7 +313,7 @@ int main() {
   write_to_locked_log((char *[]){NULL});
   write_to_locked_log((char *[]){"fcntl", NULL});
   write_to_locked_log((char *[]){"flock", NULL});
-  write_to_locked_log((char *[]){"flock", "fcntl", NULL});
+  write_to_locked_log((char *[]){"flock", "fcntl", NULL}); /* Deadlock risk! */
   write_to_locked_log((char *[]){"fcntl", "flock", NULL});
   write_concurrently();
 }

diff --git a/tl-append.c b/tl-append.c
index 1b3a9d0c6fc45572f16c775eae148a09d6d4150c..4edd58a80b271edca77f959173f9367cb1092d59 100644 (file)
--- a/tl-append.c
+++ b/tl-append.c
@@ -5,6 +5,7 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
+#include <sys/file.h>
 #include <time.h>
 #include <unistd.h>
 
@@ -22,18 +23,22 @@ const struct timespec ACKNOWLEDGE_DELAY = {0, 300000000};
 typedef struct {
   int interactive;
   int fcntl_lock;
+  int flock_lock;
 } conf_t;
 
 conf_t parse_command_line(int argc, char *argv[]) {
   conf_t conf;
   conf.interactive = 0;
   conf.fcntl_lock = 1;
+  conf.flock_lock = 1;
 
   for (int i = 1; i < argc; i++) {
     if (strcmp(argv[i], "-i") == 0 && isatty(2))
       conf.interactive = 1;
     if (strcmp(argv[i], "--no-fnctl-lock") == 0)
       conf.fcntl_lock = 0;
+    if (strcmp(argv[i], "--no-flock-lock") == 0)
+      conf.flock_lock = 0;
   }
 
   return conf;
@@ -65,7 +70,7 @@ static void write_line(const char *now, FILE *f, const char *line) {
     die("Error writing to output file");
 }
 
-static void take_lock(conf_t *conf, FILE *f) {
+static void take_fcntl_lock(conf_t *conf, FILE *f) {
   if (!conf->fcntl_lock)
     return;
   struct flock lock;
@@ -79,12 +84,34 @@ static void take_lock(conf_t *conf, FILE *f) {
   if (fcntl(fd, F_SETLK, &lock) == 0)
     return;
   if (errno != EACCES && errno != EAGAIN)
-    die_err("Couldn't take lock");
+    die_err("Couldn't take fcntl lock");
   if (fputs(WAITING, stderr) == EOF)
     die("Error writing waiting message");
   if (fcntl(fd, F_SETLKW, &lock) == 0)
     return;
-  die_err("Couldn't take lock");
+  die_err("Couldn't take fcntl lock");
+}
+
+static void take_flock_lock(conf_t *conf, FILE *f) {
+  if (!conf->flock_lock)
+    return;
+  int fd = fileno(f);
+  if (fd == -1)
+    die_err("Couldn't get file descriptor for locking");
+  if (flock(fd, LOCK_EX | LOCK_NB) == 0)
+    return;
+  if (errno != EWOULDBLOCK)
+    die_err("Couldn't take flock lock");
+  if (fputs(WAITING, stderr) == EOF)
+    die("Error writing waiting message");
+  if (flock(fd, LOCK_EX) == 0)
+    return;
+  die_err("Couldn't take flock lock");
+}
+
+static void take_lock(conf_t *conf, FILE *f) {
+  take_fcntl_lock(conf, f);
+  take_flock_lock(conf, f);
 }
 
 static void write_acknowledgment(conf_t *conf) {