Changeset: 60c53fa66e68 for MonetDB
URL: http://dev.monetdb.org/hg/MonetDB?cmd=changeset;node=60c53fa66e68
Modified Files:
        common/utils/msabaoth.c
        common/utils/mutils.c
        gdk/gdk_utils.c
        tools/merovingian/daemon/argvcmds.c
        tools/merovingian/daemon/merovingian.c
Branch: Jan2014
Log Message:

Fix up database locking.
MT_lockf returns the file descriptor of the open lock file when
locking, or 0 when unlocking (or a negative value if either operation
failed).  Unlocking does not close the file descriptor, i.e. that has
to be done separately.
Don't try to reopen the lock file.  Ever.  (It fails on Windows.)
On Windows, MT_lockf now uses the open() call which in turn uses
CreateFile.  We retrieve the CreateFile file handle with
_get_osfhandle() and use it to lock the file.  We need to do this,
since reopening the file once it is locked doesn't work.

In various places, a call to unlock the locked file was added.  It is
not strictly needed if the program exits, but it may speed up
reusability of the lock file (especially on Windows).


diffs (truncated from 366 to 300 lines):

diff --git a/common/utils/msabaoth.c b/common/utils/msabaoth.c
--- a/common/utils/msabaoth.c
+++ b/common/utils/msabaoth.c
@@ -49,6 +49,7 @@
 #if defined(_MSC_VER) && _MSC_VER >= 1400
 #define close _close
 #define unlink _unlink
+#define fdopen _fdopen
 #endif
 
 #define PATHLENGTH 4096
@@ -674,7 +675,7 @@ msab_getStatus(sabdb** ret, char *dbname
                } else {
                        /* locking succeed, check for a crash in the uplog */
                        snprintf(log, sizeof(log), "%s/%s/%s", path, e->d_name, 
UPLOGFILE);
-                       if ((f = fopen(log, "r")) != NULL) {
+                       if ((f = fdopen(fd, "r+")) != NULL) {
                                (void)fseek(f, -1, SEEK_END);
                                if (fread(data, 1, 1, f) != 1) {
                                        /* the log is empty, assume no crash */
@@ -684,11 +685,13 @@ msab_getStatus(sabdb** ret, char *dbname
                                } else { /* should be \t */
                                        sdb->state = SABdbCrashed;
                                }
+                               /* release the lock */
+                               MT_lockf(buf, F_ULOCK, 4, 1);
                                (void)fclose(f);
-                       } /* cannot happen, we checked it before */
-
-                       /* release the lock */
-                       close(fd);
+                       } else {
+                               /* shouldn't happen */
+                               close(fd);
+                       }
                }
                snprintf(buf, sizeof(buf), "%s/%s/%s", path, e->d_name,
                                MAINTENANCEFILE);
diff --git a/common/utils/mutils.c b/common/utils/mutils.c
--- a/common/utils/mutils.c
+++ b/common/utils/mutils.c
@@ -204,12 +204,16 @@ dirname(char *path)
 int
 MT_lockf(char *filename, int mode, off_t off, off_t len)
 {
-       int ret = 1, illegalmode = 0, fd = -1;
+       int ret = 1, fd = -1;
        OVERLAPPED ov;
        OSVERSIONINFO os;
-       HANDLE fh = CreateFile(filename,
-                              GENERIC_READ | GENERIC_WRITE, 0,
-                              NULL, OPEN_ALWAYS, FILE_ATTRIBUTE_NORMAL, NULL);
+       HANDLE fh;
+       static struct lockedfiles {
+               struct lockedfiles *next;
+               char *filename;
+               int fildes;
+       } *lockedfiles;
+       struct lockedfiles **fpp, *fp;
 
        os.dwOSVersionInfoSize = sizeof(OSVERSIONINFO);
        GetVersionEx(&os);
@@ -230,36 +234,59 @@ MT_lockf(char *filename, int mode, off_t
 #endif
 #endif
 
-       if (fh == NULL) {
+       if (mode == F_ULOCK) {
+               for (fpp = &lockedfiles; (fp = *fpp) != NULL; fpp = &fp->next) {
+                       if (strcmp(fp->filename, filename) == 0) {
+                               free(fp->filename);
+                               fd = fp->fildes;
+                               fh = (HANDLE) _get_osfhandle(fd);
+                               fp = *fpp;
+                               *fpp = fp->next;
+                               free(fp);
+                               ret = UnlockFileEx(fh, 0, len, 0, &ov);
+                               return ret ? 0 : -1;
+                       }
+               }
+               /* didn't find the locked file, try opening the file
+                * directly */
+               fh = CreateFile(filename,
+                               GENERIC_READ | GENERIC_WRITE, 0,
+                               NULL, OPEN_ALWAYS, FILE_ATTRIBUTE_NORMAL, NULL);
+               if (fh == INVALID_HANDLE_VALUE)
+                       return -2;
+               ret = UnlockFileEx(fh, 0, len, 0, &ov);
+               CloseHandle(fh);
+               return 0;
+       }
+
+       fd = open(filename, O_CREAT | O_RDWR | O_TEXT, MONETDB_MODE);
+       if (fd < 0)
+               return -2;
+       fh = (HANDLE) _get_osfhandle(fd);
+       if (fh == INVALID_HANDLE_VALUE) {
+               close(fd);
                return -2;
        }
-       if (mode == F_ULOCK) {
-               if (os.dwPlatformId != VER_PLATFORM_WIN32_WINDOWS)
-                       ret = UnlockFileEx(fh, 0, 0, len, &ov);
-       } else if (mode == F_TLOCK) {
-               if (os.dwPlatformId != VER_PLATFORM_WIN32_WINDOWS)
-                       ret = LockFileEx(fh, LOCKFILE_FAIL_IMMEDIATELY | 
LOCKFILE_EXCLUSIVE_LOCK, 0, 0, len, &ov);
+
+       if (mode == F_TLOCK) {
+               ret = LockFileEx(fh, LOCKFILE_FAIL_IMMEDIATELY | 
LOCKFILE_EXCLUSIVE_LOCK, 0, len, 0, &ov);
        } else if (mode == F_LOCK) {
-               if (os.dwPlatformId != VER_PLATFORM_WIN32_WINDOWS)
-                       ret = LockFileEx(fh, LOCKFILE_EXCLUSIVE_LOCK, 0, 0, 
len, &ov);
+               ret = LockFileEx(fh, LOCKFILE_EXCLUSIVE_LOCK, 0, len, 0, &ov);
        } else {
-               illegalmode = 1;
-       }
-       CloseHandle(fh);
-       if (illegalmode) {
+               close(fd);
                SetLastError(ERROR_INVALID_DATA);
+               return -2;
        }
        if (ret != 0) {
-               fd = open(filename, O_CREAT | O_RDWR, MONETDB_MODE);
-               if (fd < 0) {
-                       /* this is nasty, but I "trust" windows that it in this 
case
-                        * also cannot open the file into a filehandle any 
more, so
-                        * unlocking is in vain. */
-                       return -2;
-               } else {
-                       return fd;
+               if ((fp = malloc(sizeof(*fp))) != NULL &&
+                   (fp->filename = strdup(filename)) != NULL) {
+                       fp->fildes = fd;
+                       fp->next = lockedfiles;
+                       lockedfiles = fp;
                }
+               return fd;
        } else {
+               close(fd);
                return -1;
        }
 }
@@ -292,20 +319,25 @@ lockf(int fd, int cmd, off_t len)
 }
 #endif
 
+#ifndef O_TEXT
+#define O_TEXT 0
+#endif
 /* returns -1 when locking failed,
  * returns -2 when the lock file could not be opened/created
  * returns the (open) file descriptor to the file otherwise */
 int
 MT_lockf(char *filename, int mode, off_t off, off_t len)
 {
-       int fd = open(filename, O_CREAT | O_RDWR, MONETDB_MODE);
+       int fd = open(filename, O_CREAT | O_RDWR | O_TEXT, MONETDB_MODE);
 
        if (fd < 0)
                return -2;
 
-       if (lseek(fd, off, SEEK_SET) == off && lockf(fd, mode, len) == 0) {
+       if (lseek(fd, off, SEEK_SET) >= 0 &&
+           lockf(fd, mode, len) == 0) {
                /* do not close else we lose the lock we want */
-               return fd;
+               (void) lseek(fd, 0, SEEK_SET); /* move seek pointer back */
+               return mode == F_ULOCK ? 0 : fd;
        }
        close(fd);
        return -1;
diff --git a/gdk/gdk_utils.c b/gdk/gdk_utils.c
--- a/gdk/gdk_utils.c
+++ b/gdk/gdk_utils.c
@@ -72,7 +72,6 @@ BAT *GDKval = NULL;
 
 static volatile ATOMIC_FLAG GDKstopped = ATOMIC_FLAG_INIT;
 static void GDKunlockHome(void);
-static int GDKgetHome(void);
 
 #undef malloc
 #undef calloc
@@ -200,14 +199,13 @@ GDKlog(const char *format, ...)
 {
        va_list ap;
        char *p = 0, buf[1024];
-       int mustopen = GDKgetHome();
        time_t tm = time(0);
 #if defined(HAVE_CTIME_R3) || defined(HAVE_CTIME_R)
        char tbuf[26];
 #endif
        char *ctm;
 
-       if (MT_pagesize() == 0)
+       if (MT_pagesize() == 0 || GDKlockFile == NULL)
                return;
 
        va_start(ap, format);
@@ -235,9 +233,6 @@ GDKlog(const char *format, ...)
 #endif
        fprintf(GDKlockFile, "USR=%d PID=%d TIME=%.24s @ %s\n", (int) getuid(), 
(int) getpid(), ctm, buf);
        fflush(GDKlockFile);
-
-       if (mustopen)
-               GDKunlockHome();
 }
 
 /*
@@ -1004,6 +999,7 @@ GDKinit(opt *set, int setlen)
        _set_abort_behavior(0, _CALL_REPORTFAULT | _WRITE_ABORT_MSG);
        _set_error_mode(_OUT_TO_STDERR);
 #endif
+       /* now try to lock the database */
        GDKlockHome();
 
        /* Mserver by default takes 80% of all memory as a default */
@@ -1149,6 +1145,10 @@ GDKexiting(void)
 void
 GDKexit(int status)
 {
+       if (GDKlockFile == NULL) {
+               /* no database lock, so no threads, so exit now */
+               exit(status);
+       }
        if (ATOMIC_TAS(GDKstopped, GDKstoppedLock, "GDKexit") == 0) {
                MT_Id pid = MT_getpid();
                Thread t, s;
@@ -1185,7 +1185,6 @@ GDKexit(int status)
                        }
                        MT_lock_unset(&GDKthreadLock, "GDKexit");
                }
-               (void) GDKgetHome();
 #if 0
                /* we can't clean up after killing threads */
                BBPexit();
@@ -1227,8 +1226,9 @@ MT_Lock GDKtmLock MT_LOCK_INITIALIZER("G
 static void
 GDKlockHome(void)
 {
-       char *p = 0, buf[1024], host[PATHLENGTH];
+       int fd;
 
+       assert(GDKlockFile == NULL);
        /*
         * Go there and obtain the global database lock.
         */
@@ -1244,21 +1244,16 @@ GDKlockHome(void)
                        GDKfatal("GDKlockHome: could not move to %s\n", 
GDKdbpathStr);
                IODEBUG THRprintf(GDKstdout, "#GDKlockHome: created directory 
%s\n", GDKdbpathStr);
        }
-       if (MT_lockf(GDKLOCK, F_TLOCK, 4, 1) < 0) {
-               GDKlockFile = 0;
+       if ((fd = MT_lockf(GDKLOCK, F_TLOCK, 4, 1)) < 0) {
                GDKfatal("GDKlockHome: Database lock '%s' denied\n", GDKLOCK);
        }
-       if ((GDKlockFile = fopen(GDKLOCK, "rb+")) == NULL) {
+
+       /* now we have the lock on the database */
+
+       if ((GDKlockFile = fdopen(fd, "r+")) == NULL) {
+               close(fd);
                GDKfatal("GDKlockHome: Could not open %s\n", GDKLOCK);
        }
-       if (fgets(buf, 1024, GDKlockFile) && (p = strchr(buf, ':')))
-               *p = 0;
-       if (p) {
-               sprintf(host, " from '%s'", buf);
-       } else {
-               IODEBUG THRprintf(GDKstdout, "#GDKlockHome: ignoring empty or 
invalid %s.\n", GDKLOCK);
-               host[0] = 0;
-       }
        /*
         * We have the lock, are the only process currently allowed in
         * this section.
@@ -1290,28 +1285,8 @@ GDKunlockHome(void)
 }
 
 /*
- * Really really get the lock. Now!!
- */
-static int
-GDKgetHome(void)
-{
-       if (MT_pagesize() == 0 || GDKlockFile)
-               return 0;
-       while ((GDKlockFile = fopen(GDKLOCK, "r+")) == NULL) {
-               GDKerror("GDKgetHome: PANIC on open %s. sleep(1)\n", GDKLOCK);
-               MT_sleep_ms(1000);
-       }
-       if (MT_lockf(GDKLOCK, F_TLOCK, 4, 1) < 0) {
-               IODEBUG THRprintf(GDKstdout, "#GDKgetHome: blocking on lock 
'%s'.\n", GDKLOCK);
-               MT_lockf(GDKLOCK, F_LOCK, 4, 1);
-       }
-       return 1;
-}
-
-
-/*
  * @+ Error handling
_______________________________________________
checkin-list mailing list
[email protected]
https://www.monetdb.org/mailman/listinfo/checkin-list

Reply via email to