Re: [Nmh-workers] dot locking broken?

[Marcin Cieslak]

>> David Levine <address@hidden> wrote:
>> >> I was also wondering if we should give the user to abort waiting
>> >> for a lock with ^C.
>> >
>> > I tried, and ^C works for me (on Linux).
>>
>> That's interesting, from what I see in the inc code SIGINT is
>> trapped in inc.c, lines 513++ ?
>
> And that code is conditional on trnflag.  I was testing with -file,
> which didn't use it.  It looks like the signal handlers are there
> to avoid corruption when the mailbox would be modified.  I think
> they should be retained.

Before we successfuly lock I think we cannot corrupt anything,
so my idea to improve this is something along the lines below.

Tested with dotlocking on FreeBSD and "touch /var/mail/<inbox>.lock".

It can take up to 3 minutes to remove a stale lockfile, so I think
it should be interruptible.

commit dcccd84c0b2543547789ad316432f4ecc9358cc5
Author: Marcin Cieslak <address@hidden>
Date:   Fri Feb 13 09:33:37 2015 +0000

    Interrupt waiting for lock on signal

diff --git a/sbr/lock_file.c b/sbr/lock_file.c
index a8a9811..6ea34cb 100644
--- a/sbr/lock_file.c
+++ b/sbr/lock_file.c
@@ -25,6 +25,7 @@
 #endif
 #include <time.h>
 #include <fcntl.h>
+#include <setjmp.h>
 #ifdef HAVE_FLOCK
 # include <sys/file.h>
 #endif
@@ -87,6 +88,12 @@ static struct lock *l_top = NULL;
 
 static int lkopen(const char *, int, mode_t, enum locktype, int *);
 static int str2accbits(const char *);
+#if !defined(HAVE_LIBLOCKFILE)
+static jmp_buf intr_buf;
+static void intrser(int sig);
+static SIGNAL_HANDLER oldsigint;
+static SIGNAL_HANDLER oldsigterm;
+#endif
 
 static int lkopen_fcntl (const char *, int, mode_t, int *);
 #ifdef HAVE_LOCKF
@@ -495,37 +502,45 @@ lkopen_dot (const char *file, int access, mode_t mode, 
int *failed_to_lock)
 #if !defined(HAVE_LIBLOCKFILE)
     {
        int i;
-       for (i = 0; i < LOCK_RETRIES; ++i) {
-           /* attempt to create lock file */
-           if (lockit (&lkinfo) == 0) {
-               /* if successful, turn on timer and return */
-               timerON (lkinfo.curlock, fd);
-               return fd;
-           } else {
-               /*
-                * Abort locking, if we fail to lock after 5 attempts
-                * and are never able to stat the lock file.  Or, if
-                * we can stat the lockfile but exceed LOCK_RETRIES
-                * seconds waiting for it (by falling out of the loop).
-                */
-               struct stat st;
-               if (stat (lkinfo.curlock, &st) == -1) {
-                   if (i++ > 5) break;
-                   sleep (1);
+       if (setjmp(intr_buf) == 0) {
+           oldsigint = signal(SIGINT, intrser);
+           oldsigterm = signal(SIGTERM, intrser);
+           for (i = 0; i < LOCK_RETRIES; ++i) {
+               /* attempt to create lock file */
+               if (lockit (&lkinfo) == 0) {
+                   /* if successful, turn on timer and return */
+                   timerON (lkinfo.curlock, fd);
+                   signal(SIGINT, oldsigint);
+                   signal(SIGTERM, oldsigterm);
+                   return fd;
                } else {
-                   time_t curtime;
-                   time (&curtime);
-                   
-                   /* check for stale lockfile, else sleep */
-                   if (curtime > st.st_ctime + RSECS)
-                       (void) m_unlink (lkinfo.curlock);
-                   else
+                   /*
+                    * Abort locking, if we fail to lock after 5 attempts
+                    * and are never able to stat the lock file.  Or, if
+                    * we can stat the lockfile but exceed LOCK_RETRIES
+                    * seconds waiting for it (by falling out of the loop).
+                    */
+                   struct stat st;
+                   if (stat (lkinfo.curlock, &st) == -1) {
+                       if (i++ > 5) break;
                        sleep (1);
+                   } else {
+                       time_t curtime;
+                       time (&curtime);
+                       
+                       /* check for stale lockfile, else sleep */
+                       if (curtime > st.st_ctime + RSECS)
+                           (void) m_unlink (lkinfo.curlock);
+                       else
+                           sleep (1);
+                   }
+                   lockname (file, &lkinfo, 1);
                }
-               lockname (file, &lkinfo, 1);
            }
        }
 
+       signal(SIGINT, oldsigint);
+       signal(SIGTERM, oldsigterm);
         *failed_to_lock = 1;
         return -1;
     }
@@ -772,3 +787,14 @@ init_locktype(const char *lockname)
        return 0;
     }
 }
+
+#if !defined(HAVE_LIBLOCKFILE)
+/*
+** Handle signal
+*/
+static void
+intrser(int sig)
+{
+       longjmp(intr_buf, 1);
+}
+#endif