New modes, including a retry mode (the default), removed fcntl

and stub read/write lock mode.
New function, to remove a lock even if not owned (useful for the
external dotlock utility).
New functions, to set the expire time for MU_LOCKER_TIME, the flags,
and the retries and retry sleep for MU_LOCKER_RETRY.

@@ -28,23 +28,49 @@ extern "C" {
 struct _locker;
 struct _locker;
 typedef struct _locker *locker_t;
 typedef struct _locker *locker_t;
 
 
-extern int locker_create   __P ((locker_t *, const char *filename,
-				 size_t len, int flags));
+/* lock expiry time */
+#define MU_LOCKER_EXPIRE_TIME        (10 * 60)
+#define MU_LOCKER_RETRIES            (10)
+#define MU_LOCKER_RETRY_SLEEP        (1)
+
+/* locker_create() flags */
+
+#define MU_LOCKER_RETRY    0x01
+  /* This requests that we loop retries times, sleeping retry_sleep
+     seconds in between trying to obtain the lock before failing with
+     MU_LOCK_CONFLICT. */
+#define MU_LOCKER_TIME     0x02
+  /* This mode checks the last update time of the lock, then removes
+     it if older than MU_LOCKER_EXPIRE_TIME. If a client uses this,
+     then the servers better periodically update the lock on the
+     file... do they? */
+#define MU_LOCKER_PID      0x04
+  /* PID locking is only useful for programs that aren't using
+     an external dotlocker, non-setgid programs will use a dotlocker,
+     which locks and exits imediately. This is a protection against
+     a server crashing, it's not generally useful. */
+
+#define MU_LOCKER_DEFAULT  (MU_LOCKER_RETRY)
+
+extern int locker_create __P ((locker_t *, const char *filename, int flags));
 extern void locker_destroy __P ((locker_t *));
 extern void locker_destroy __P ((locker_t *));
 
 
-#define MU_LOCKER_RDLOCK 0
-#define MU_LOCKER_WRLOCK 1
+/* Time is measured in seconds. */
 
 
-/* locking flags */
-#define MU_LOCKER_PID    1
-#define MU_LOCKER_FCNTL  2
-#define MU_LOCKER_TIME   4
+extern int locker_set_flags __P ((locker_t, int));
+extern int locker_set_expire_time __P ((locker_t, int));
+extern int locker_set_retries __P ((locker_t, int));
+extern int locker_set_retry_sleep __P ((locker_t, int));
 
 
-#define MU_LOCKER_EXPIRE_TIME        (5 * 60)
+extern int locker_get_flags __P ((locker_t, int*));
+extern int locker_get_expire_time __P ((locker_t, int*));
+extern int locker_get_retries __P ((locker_t, int*));
+extern int locker_get_retry_sleep __P ((locker_t, int*));
 
 
-extern int locker_lock          __P ((locker_t, int flag));
+extern int locker_lock          __P ((locker_t));
 extern int locker_touchlock     __P ((locker_t));
 extern int locker_touchlock     __P ((locker_t));
 extern int locker_unlock        __P ((locker_t));
 extern int locker_unlock        __P ((locker_t));
+extern int locker_remove_lock   __P ((locker_t));
 
 
 #ifdef __cplusplus
 #ifdef __cplusplus
 }
 }

@@ -19,61 +19,104 @@
 # include <config.h>
 # include <config.h>
 #endif
 #endif
 
 
+#include <assert.h>
 #include <errno.h>
 #include <errno.h>
-#include <sys/types.h>
-#include <stdlib.h>
-#include <string.h>
-#include <stdio.h>
 #include <fcntl.h>
 #include <fcntl.h>
 #include <limits.h>
 #include <limits.h>
-#include <sys/stat.h>
-#include <unistd.h>
+#include <signal.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
 #include <time.h>
 #include <time.h>
+#include <unistd.h>
 #include <utime.h>
 #include <utime.h>
-#include <signal.h>
+
+#include <sys/types.h>
+#include <sys/stat.h>
 
 
 #include <mailutils/locker.h>
 #include <mailutils/locker.h>
+#include <mailutils/errno.h>
 
 
 #define LOCKFILE_ATTR           0444
 #define LOCKFILE_ATTR           0444
 
 
 /* First draft by Brian Edmond. */
 /* First draft by Brian Edmond. */
+/* For subsequent modifications, see the GNU mailutils ChangeLog. */
 
 
 struct _locker
 struct _locker
 {
 {
   int fd;
   int fd;
   int refcnt;
   int refcnt;
-  char *fname;
+
+  char *file;
+
+  char *dotlock;
+
   int flags;
   int flags;
+
+  int expire_time;
+  int retries;
+  int retry_sleep;
 };
 };
 
 
+/* Assert that we're managing the refcnt and fd correctly, either
+ * we have a lock, and the fd is valid, or refcnt is 0 and fd is -1.
+ * And refcnt can never be less than 0.
+ */
+#define INVARIANT(l) \
+  assert((l)->refcnt >= 0); \
+  assert( \
+      (((l)->refcnt >  0) && ((l)->fd != -1)) || \
+      (((l)->refcnt == 0) && ((l)->fd == -1)) \
+      );
+
 int
 int
-locker_create (locker_t *plocker, const char *filename, size_t len, int flags)
+locker_create (locker_t *plocker, const char *filename, int flags)
 {
 {
   locker_t l;
   locker_t l;
 
 
-  if (plocker == NULL || filename == NULL || len == 0)
+  if (plocker == NULL)
+    return MU_ERR_OUT_PTR_NULL;
+
+  if(filename == NULL)
     return EINVAL;
     return EINVAL;
 
 
-  l = malloc (sizeof (*l));
+  l = calloc (1, sizeof (*l));
   if (l == NULL)
   if (l == NULL)
     return ENOMEM;
     return ENOMEM;
 
 
-  l->fname = calloc (len + 5 /*strlen(".lock")*/ + 1, sizeof (*(l->fname)));
-  if (l->fname == NULL)
+  /* Should make l->file be the resulting of following the symlinks. */
+  l->file = strdup(filename);
+  if (l->file == NULL)
     {
     {
       free (l);
       free (l);
       return ENOMEM;
       return ENOMEM;
     }
     }
-  memcpy (l->fname, filename, len);
-  strcat (l->fname, ".lock");
+
+  l->dotlock = malloc(strlen(l->file) + 5 /*strlen(".lock")*/ + 1);
+
+  if(!l->dotlock)
+  {
+    free(l->file);
+    free(l);
+    return ENOMEM;
+  }
+
+  sprintf(l->dotlock, "%s.lock", l->file);
 
 
   if (flags)
   if (flags)
     l->flags = flags;
     l->flags = flags;
   else
   else
-    l->flags = MU_LOCKER_TIME; /* Default is time lock implementation.  */
+    l->flags = MU_LOCKER_DEFAULT;
+
   l->fd = -1;
   l->fd = -1;
-  l->refcnt = 0;
+  l->expire_time = MU_LOCKER_EXPIRE_TIME;
+  l->retries = MU_LOCKER_RETRIES;
+  l->retry_sleep = MU_LOCKER_RETRY_SLEEP;
+
+  INVARIANT(l)
+
   *plocker = l;
   *plocker = l;
+
   return 0;
   return 0;
 }
 }
 
 
@@ -82,151 +125,387 @@ locker_destroy (locker_t *plocker)
 {
 {
   if (plocker && *plocker)
   if (plocker && *plocker)
     {
     {
-      free ((*plocker)->fname);
+      close((*plocker)->fd);
+      free ((*plocker)->file);
+      free ((*plocker)->dotlock);
       free (*plocker);
       free (*plocker);
       *plocker = NULL;
       *plocker = NULL;
     }
     }
 }
 }
 
 
+int locker_set_flags (locker_t locker, int flags)
+{
+  if (!locker)
+    return MU_ERR_LOCKER_NULL;
+
+  locker->flags = flags;
+
+  return 0;
+}
+
 int
 int
-locker_lock (locker_t lock, int flags)
+locker_set_expire_time (locker_t locker, int time)
 {
 {
-  int   fd = -1;
-  char  buf[16];
-  pid_t pid;
-  int   removed = 0;
+  if (!locker)
+    return MU_ERR_LOCKER_NULL;
+
+  if(time <= 0)
+    return EINVAL;
+
+  locker->expire_time = time;
+
+  return 0;
+}
+
+int
+locker_set_retries (locker_t locker, int retries)
+{
+  if (!locker)
+    return MU_ERR_LOCKER_NULL;
+
+  if(retries <= 0)
+    return EINVAL;
+
+  locker->retries = retries;
+
+  return 0;
+}
+
+int
+locker_set_retry_sleep (locker_t locker, int retry_sleep)
+{
+  if (!locker)
+    return MU_ERR_LOCKER_NULL;
+
+  if(retry_sleep <= 0)
+    return EINVAL;
+
+  locker->retry_sleep = retry_sleep;
+
+  return 0;
+}
+
+int locker_get_flags (locker_t locker, int *flags)
+{
+  if (!locker)
+    return MU_ERR_LOCKER_NULL;
+
+  if(!flags)
+    return EINVAL;
+
+  *flags = locker->flags;
+
+  return 0;
+}
+
+int
+locker_get_expire_time (locker_t locker, int *time)
+{
+  if (!locker)
+    return MU_ERR_LOCKER_NULL;
+
+  if(!time)
+    return EINVAL;
+
+  *time = locker->expire_time;
+
+  return 0;
+}
+
+int
+locker_get_retries (locker_t locker, int *retries)
+{
+  if (!locker)
+    return MU_ERR_LOCKER_NULL;
+
+  if(!retries)
+    return EINVAL;
+
+  *retries = locker->retries;
+
+  return 0;
+}
+
+int
+locker_get_retry_sleep (locker_t locker, int *retry_sleep)
+{
+  if (!locker)
+    return MU_ERR_LOCKER_NULL;
+
+  if(!retry_sleep)
+    return EINVAL;
+
+  *retry_sleep = locker->retry_sleep;
+
+  return 0;
+}
+
+/* expire a stale lock (if MU_LOCKER_PID or MU_LOCKER_TIME) */
+static void
+expire_stale_lock (locker_t lock)
+{
+  int stale = 0;
+  int fd = open (lock->dotlock, O_RDONLY);
+  if (fd == -1)
+    return;
+
+  /* Check to see if this process is still running.  */
+  if (lock->flags & MU_LOCKER_PID)
+    {
+      char buf[16];
+      pid_t pid;
+      int nread = read (fd, buf, sizeof (buf) - 1);
+      if (nread > 0)
+	{
+	  buf[nread] = '\0';
+	  pid = strtol (buf, NULL, 10);
+	  if (pid > 0)
+	    {
+	      /* Process is gone so we try to remove the lock. */
+	      if (kill (pid, 0) == -1)
+		stale = 1;
+	    }
+	  else
+	    stale = 1;		/* Corrupted file, remove the lock. */
+	}
+    }
+  /* Check to see if the lock expired.  */
+  if (lock->flags & MU_LOCKER_TIME)
+    {
+      struct stat stbuf;
+
+      fstat (fd, &stbuf);
+      /* The lock has expired. */
+      if ((time (NULL) - stbuf.st_mtime) > lock->expire_time)
+	stale = 1;
+    }
+
+  close (fd);
+  if (stale)
+    unlink (lock->dotlock);
+}
+
+static int
+stat_check (const char *file, int fd)
+{
+  struct stat fn_stat;
+  struct stat fd_stat;
+  int err = 0;
+
+  /* We should always be able to stat a valid fd, so this
+     is an error condition. */
+  if (lstat (file, &fn_stat) || fstat (fd, &fd_stat))
+    err = errno;
+  else
+    {
+      /* If the link and stat don't report the same info, or the
+         file is a symlink, fail the locking. */
+#define CHK(X) if(X) return EINVAL
+
+      CHK (!S_ISREG (fn_stat.st_mode));
+      CHK (!S_ISREG (fd_stat.st_mode));
+      CHK (fn_stat.st_nlink != 1);
+      CHK (fn_stat.st_dev != fd_stat.st_dev);
+      CHK (fn_stat.st_ino != fd_stat.st_ino);
+      CHK (fn_stat.st_mode != fd_stat.st_mode);
+      CHK (fn_stat.st_nlink != fd_stat.st_nlink);
+      CHK (fn_stat.st_uid != fd_stat.st_uid);
+      CHK (fn_stat.st_gid != fd_stat.st_gid);
+      CHK (fn_stat.st_rdev != fd_stat.st_rdev);
+
+#undef CHK
+    }
+  return err;
+}
+
+int
+locker_lock (locker_t lock)
+{
+  int err = 0;
+  int fd = -1;
+  int retries = 1;
 
 
-  (void)flags; /* Ignore for now.  */
   if (lock == NULL)
   if (lock == NULL)
     return EINVAL;
     return EINVAL;
 
 
-  /* Is the lock already applied?
-     FIXME: should we check flags != lock->flags ?? */
-  if (lock->fd != -1)
+  INVARIANT (lock)
+    /* Is the lock already applied? */
+    if (lock->refcnt > 0)
     {
     {
+      assert (lock->fd != -1);
       lock->refcnt++;
       lock->refcnt++;
       return 0;
       return 0;
     }
     }
 
 
-  /*
-    Check for lock existance:
-    if it exists but the process is gone the lock can be removed,
-    if the lock is expired, remove it.  */
-  fd = open (lock->fname, O_RDONLY);
-  if (fd != -1)
-    {
-      /* Check to see if this process is still running.  */
-      if (lock->flags & MU_LOCKER_PID)
-        {
-	  int nread = read (fd, buf, sizeof (buf) - 1);
-          if (nread > 0)
-            {
-	      buf[nread] = '\0';
-              pid = strtol (buf, NULL, 10);
-              if (pid > 0)
-                {
-                  /* Process is gone so we try to remove the lock.  */
-                  if (kill (pid, 0) == -1)
-                    removed = 1;
-                }
-	      else
-		removed = 1; /* Corrupted file, remove the lock.  */
-            }
-        }
-      /* Check to see if the lock expired.  */
-      if (lock->flags & MU_LOCKER_TIME)
-        {
-          struct stat stbuf;
-
-          fstat (fd, &stbuf);
-          /* The lock has expired.  */
-          if ((time (NULL) - stbuf.st_mtime) > MU_LOCKER_EXPIRE_TIME)
-            removed = 1;
-        }
+  assert (lock->fd == -1);
 
 
+  /* Check we are trying to lock a regular file, with a link count
+     of 1, that we have permission to read, etc., or don't lock it. */
+  if ((fd = open (lock->file, O_RDONLY)) == -1)
+    {
+      return errno;
+    }
+  else
+    {
+      err = stat_check (lock->file, fd);
       close (fd);
       close (fd);
-      if (removed)
-        unlink (lock->fname);
+      fd = -1;
+      if (err)
+	{
+	  if (err == EINVAL)
+	    err = MU_ERR_LOCK_BAD_FILE;
+	  return err;
+	}
     }
     }
 
 
-  /* Try to create the lockfile.  */
-  fd = open (lock->fname, O_WRONLY | O_CREAT | O_EXCL, LOCKFILE_ATTR);
-  if (fd == -1)
-    return errno;
-  else
+  if (lock->flags & MU_LOCKER_RETRY)
+    {
+      retries = lock->retries;
+    }
+
+  while (retries--)
     {
     {
-      struct stat fn_stat;
-      struct stat fd_stat;
-
-      if (lstat (lock->fname, &fn_stat)
-	  || fstat(fd, &fd_stat)
-	  || fn_stat.st_nlink != 1
-	  || fn_stat.st_dev != fd_stat.st_dev
-	  || fn_stat.st_ino != fd_stat.st_ino
-	  || fn_stat.st_uid != fd_stat.st_uid
-	  || fn_stat.st_gid != fd_stat.st_gid)
+      /* cleanup after last loop */
+      close (fd);
+      fd = -1;
+
+      expire_stale_lock (lock);
+
+      /* Try to create the lockfile. */
+      fd = open (lock->dotlock, O_WRONLY | O_CREAT | O_EXCL, LOCKFILE_ATTR);
+      if (fd == -1)
 	{
 	{
-	  close (fd);
-	  unlink (lock->fname);
-	  return EPERM;
+	  err = errno;
+
+	  /* EEXIST means somebody else has the lock, anything else is an
+	     error. */
+	  if (err == EEXIST)
+	    err = MU_ERR_LOCK_CONFLICT;
+	  else
+	    return err;
 	}
 	}
+      else
+	{
+	  err = stat_check (lock->dotlock, fd);
+	  if (err == 0)
+	    {
+	      /* We got the lock! */
+	      break;
+	    }
+	  else
+	    {
+	      close (fd);
+	      unlink (lock->dotlock);
+
+	      /* If there was something invalid about the file/fd, return
+	         a more descriptive code. */
+	      if (err == EINVAL)
+		err = MU_ERR_LOCK_BAD_LOCK;
+
+	      return err;
+	    }
+	}
+      if(retries)
+	sleep(lock->retry_sleep);
     }
     }
 
 
-  /* Success.  */
-  sprintf (buf, "%ld", (long)getpid ());
-  write (fd, buf, strlen (buf));
+  if (err)
+    return err;
+
+  /* We have the lock. */
+  assert (lock->refcnt == 0);
 
 
-  /* Try to get a file lock.  */
-  if (lock->flags & MU_LOCKER_FCNTL)
+  if (lock->flags & MU_LOCKER_PID)
     {
     {
-      struct flock fl;
-
-      memset (&fl, 0, sizeof (struct flock));
-      fl.l_type = F_WRLCK;
-      if (fcntl (fd, F_SETLK, &fl) == -1)
-        {
-	  int err = errno;
-          /* Could not get the file lock.  */
-          close (fd);
-          unlink (lock->fname); /* Remove the file I created.  */
-          return err;
-        }
+      char buf[16];
+      sprintf (buf, "%ld", (long) getpid ());
+      write (fd, buf, strlen (buf));
     }
     }
 
 
+  lock->refcnt = 1;
   lock->fd = fd;
   lock->fd = fd;
-  lock->refcnt++;
+
+  INVARIANT (lock);
+
   return 0;
   return 0;
 }
 }
 
 
 int
 int
 locker_touchlock (locker_t lock)
 locker_touchlock (locker_t lock)
 {
 {
- if (!lock || !lock->fname || lock->fd == -1)
-    return EINVAL;
-  return utime (lock->fname, NULL);
+  if (!lock)
+    return MU_ERR_LOCKER_NULL;
+
+  assert(lock->dotlock);
+
+  INVARIANT(lock);
+
+  if(lock->refcnt > 0)
+    return utime (lock->dotlock, NULL);
+
+  return MU_ERR_LOCK_NOT_HELD;
 }
 }
 
 
 int
 int
 locker_unlock (locker_t lock)
 locker_unlock (locker_t lock)
 {
 {
-  if (!lock || !lock->fname || lock->fd == -1 || lock->refcnt <= 0)
-    return EINVAL;
+  assert(lock->refcnt >= 0);
+
+  /* FIXME: distinguish between bad args and lock not held */
+  if (!lock)
+    return MU_ERR_LOCKER_NULL;
+
+  INVARIANT(lock);
+
+  if(lock->refcnt == 0)
+    return MU_ERR_LOCK_NOT_HELD;
+
+  assert(lock->fd != -1);
 
 
   if (--lock->refcnt > 0)
   if (--lock->refcnt > 0)
     return 0;
     return 0;
 
 
-  if (lock->flags & MU_LOCKER_FCNTL)
-    {
-      struct flock fl;
+  assert(lock->refcnt == 0);
 
 
-      memset (&fl, 0, sizeof (struct flock));
-      fl.l_type = F_UNLCK;
-      /* Unlock failed?  */
-      if (fcntl (lock->fd, F_SETLK, &fl) == -1)
-          return errno;
-    }
   close (lock->fd);
   close (lock->fd);
   lock->fd = -1;
   lock->fd = -1;
-  unlink (lock->fname);
+  unlink (lock->dotlock);
+
+  INVARIANT(lock);
+
   return 0;
   return 0;
 }
 }
+int
+locker_remove_lock (locker_t lock)
+{
+  int err;
+
+  if (!lock)
+    return MU_ERR_LOCKER_NULL;
+
+  INVARIANT(lock);
+
+  /* If we hold the lock, do an unlock... */
+  if(lock->refcnt > 0)
+  {
+    /* Force the reference count to 1 to unlock the file. */
+    lock->refcnt = 1;
+    return locker_unlock(lock);
+  }
+
+  /* ... if we don't, unlink the lockfile. */
+  err = unlink (lock->dotlock);
+
+  if(err == -1)
+  {
+    err = errno;
+    
+    if(err == ENOENT)
+      err = MU_ERR_LOCK_NOT_HELD;
+  }
+
+  INVARIANT(lock);
+
+  return err;
+}
+