Add tests for singlesinstance()

src/calibre/utils/lock.py

@@ -9,17 +9,19 @@ import atexit
 import errno
 import os
 import stat
+import tempfile
 import time
+from functools import partial
 
 from calibre.constants import (
-    __appname__, fcntl, filesystem_encoding, ishaiku, islinux, iswindows, win32api,
+    __appname__, fcntl, filesystem_encoding, islinux, isosx, iswindows
-    win32event
 )
 from calibre.utils.monotonic import monotonic
 
 if iswindows:
+    import msvcrt, win32file, pywintypes, winerror, win32api, win32event
+    from calibre.constants import get_windows_username
     excl_file_mode = stat.S_IREAD | stat.S_IWRITE
-    import msvcrt, win32file, pywintypes, winerror
 else:
     excl_file_mode = stat.S_IRUSR | stat.S_IWUSR | stat.S_IRGRP | stat.S_IROTH
 
@@ -52,7 +54,8 @@ def windows_open(path):
     try:
         h = win32file.CreateFile(
             path,
-            win32file.GENERIC_READ | win32file.GENERIC_WRITE,  # Open for reading and writing
+            win32file.GENERIC_READ |
+            win32file.GENERIC_WRITE,  # Open for reading and writing
             0,  # Open exclusive
             None,  # No security attributes, ensures handle is not inherited by children
             win32file.OPEN_ALWAYS,  # If file does not exist, create it
@@ -66,7 +69,9 @@ def windows_open(path):
 
 
 def windows_retry(err):
-    return err.winerror in (winerror.ERROR_SHARING_VIOLATION, winerror.ERROR_LOCK_VIOLATION)
+    return err.winerror in (
+        winerror.ERROR_SHARING_VIOLATION, winerror.ERROR_LOCK_VIOLATION
+    )
 
 
 def retry_for_a_time(timeout, sleep_time, func, error_retry, *args):
@@ -108,36 +113,42 @@ class ExclusiveFile(object):
         self.file.close()
 
 
-def _clean_lock_file(file):
+def _clean_lock_file(file_obj):
     try:
-        file.close()
+        os.remove(file_obj.name)
-    except:
+    except EnvironmentError:
         pass
     try:
-        os.remove(file.name)
+        file_obj.close()
-    except:
+    except EnvironmentError:
         pass
 
 
 if iswindows:
 
-    def singleinstance(name):
+    def create_single_instance_mutex(name, per_user=True):
-        mutexname = 'mutexforsingleinstanceof' + __appname__ + name
+        mutexname = '{}-singleinstance-{}-{}'.format(
+            __appname__, (get_windows_username() if per_user else ''), name
+        )
         mutex = win32event.CreateMutex(None, False, mutexname)
+        if not mutex:
+            return
         err = win32api.GetLastError()
         if err == winerror.ERROR_ALREADY_EXISTS:
             # Close this handle other wise this handle will prevent the mutex
             # from being deleted when the process that created it exits.
             win32api.CloseHandle(mutex)
-        elif mutex and err != winerror.ERROR_INVALID_HANDLE:
+            return
-            atexit.register(win32api.CloseHandle, mutex)
+        return partial(win32api.CloseHandle, mutex)
-        return not err == winerror.ERROR_ALREADY_EXISTS
+
 elif islinux:
 
-    def singleinstance(name):
+    def create_single_instance_mutex(name, per_user=True):
         import socket
         from calibre.utils.ipc import eintr_retry_call
-        name = '%s-singleinstance-%s-%d' % (__appname__, name, os.geteuid())
+        name = '%s-singleinstance-%s-%s' % (
+            __appname__, (os.geteuid() if per_user else ''), name
+        )
         if not isinstance(name, bytes):
             name = name.encode('utf-8')
         address = b'\0' + name.replace(b' ', b'_')
@@ -146,49 +157,43 @@ elif islinux:
             eintr_retry_call(sock.bind, address)
         except socket.error as err:
             if getattr(err, 'errno', None) == errno.EADDRINUSE:
-                return False
+                return
             raise
         fd = sock.fileno()
         old_flags = fcntl.fcntl(fd, fcntl.F_GETFD)
         fcntl.fcntl(fd, fcntl.F_SETFD, old_flags | fcntl.FD_CLOEXEC)
-        atexit.register(sock.close)
+        return sock.close
-        return True
-elif ishaiku:
 
-    def singleinstance(name):
-        # Somebody should fix this.
-        return True
 else:
 
-    def singleinstance_path(name):
+    def singleinstance_path(name, per_user=True):
-        home = os.path.expanduser('~')
+        name = '%s-singleinstance-%s-%s.lock' % (
-        if os.access(home, os.W_OK | os.R_OK | os.X_OK):
+            __appname__, (os.geteuid() if per_user else ''), name
-            basename = __appname__ + '_' + name + '.lock'
-            return os.path.expanduser('~/.' + basename)
-        import tempfile
-        tdir = tempfile.gettempdir()
-        return os.path.join(
-            tdir, '%s_%s_%s.lock' % (__appname__, name, os.geteuid())
         )
+        home = os.path.expanduser('~')
+        locs = ['/var/lock', home, tempfile.gettempdir()]
+        if isosx:
+            locs.insert(0, '/Library/Caches')
+        for loc in locs:
+            if os.access(loc, os.W_OK | os.R_OK | os.X_OK):
+                return os.path.join(loc, ('.' if loc is home else '') + name)
+        raise EnvironmentError('Failed to find a suitable filesystem location for the lock file')
 
-    def singleinstance(name):
+    def create_single_instance_mutex(name, per_user=True):
-        '''
-        Return True if no other instance of the application identified by name is running,
-        False otherwise.
-        @param name: The name to lock.
-        @type name: string
-        '''
         from calibre.utils.ipc import eintr_retry_call
-        path = singleinstance_path(name)
+        path = singleinstance_path(name, per_user)
-        f = open(path, 'w')
+        f = lopen(path, 'w')
-        old_flags = fcntl.fcntl(f.fileno(), fcntl.F_GETFD)
-        fcntl.fcntl(f.fileno(), fcntl.F_SETFD, old_flags | fcntl.FD_CLOEXEC)
         try:
             eintr_retry_call(fcntl.lockf, f.fileno(), fcntl.LOCK_EX | fcntl.LOCK_NB)
-            atexit.register(_clean_lock_file, f)
+            return partial(_clean_lock_file, f)
-            return True
+        except EnvironmentError as err:
-        except IOError as err:
+            if err.errno not in (errno.EAGAIN, errno.EACCES):
-            if err.errno == errno.EAGAIN:
-                return False
                 raise
+
+
+def singleinstance(name):
+    release_mutex = create_single_instance_mutex(name)
+    if release_mutex is None:
         return False
+    atexit.register(release_mutex)
+    return True

src/calibre/utils/test_lock.py

@@ -14,7 +14,7 @@ import unittest
 from threading import Thread
 
 from calibre.constants import fcntl, iswindows
-from calibre.utils.lock import ExclusiveFile, unix_open
+from calibre.utils.lock import ExclusiveFile, unix_open, create_single_instance_mutex
 
 
 def FastFailEF(name):
@@ -37,13 +37,19 @@ def run_worker(mod, func, **kw):
     try:
         exe = [sys.executable, os.path.join(sys.setup_dir, 'run-calibre-worker.py')]
     except AttributeError:
-        exe = [os.path.join(os.path.dirname(os.path.abspath(sys.executable)), 'calibre-parallel' + ('.exe' if iswindows else ''))]
+        exe = [
+            os.path.join(
+                os.path.dirname(os.path.abspath(sys.executable)),
+                'calibre-parallel' + ('.exe' if iswindows else '')
+            )
+        ]
     env = kw.get('env', os.environ.copy())
     env['CALIBRE_SIMPLE_WORKER'] = mod + ':' + func
     if iswindows:
         import win32process
         kw['creationflags'] = win32process.CREATE_NO_WINDOW
-    kw['env'] = {str(k):str(v) for k, v in env.iteritems()}  # windows needs bytes in env
+    kw['env'] = {str(k): str(v)
+                 for k, v in env.iteritems()}  # windows needs bytes in env
     return subprocess.Popen(exe, **kw)
 
 
@@ -99,6 +105,23 @@ class IPCLockTest(unittest.TestCase):
     def test_exclusive_file_other_process_kill(self):
         self.run_other_ef_op(False)
 
+    def test_single_instance(self):
+        release_mutex = create_single_instance_mutex('test')
+        for i in range(5):
+            child = run_worker('calibre.utils.test_lock', 'other2')
+            self.assertEqual(child.wait(), 0)
+        release_mutex()
+        for i in range(5):
+            child = run_worker('calibre.utils.test_lock', 'other2')
+            self.assertEqual(child.wait(), 1)
+        child = run_worker('calibre.utils.test_lock', 'other3')
+        while not os.path.exists('ready'):
+            time.sleep(0.01)
+        child.kill()
+        release_mutex = create_single_instance_mutex('test')
+        self.assertIsNotNone(release_mutex)
+        release_mutex()
+
 
 def other1():
     e = ExclusiveFile('test')
@@ -108,6 +131,17 @@ def other1():
             time.sleep(0.02)
 
 
+def other2():
+    release_mutex = create_single_instance_mutex('test')
+    raise SystemExit(0 if release_mutex is None else 1)
+
+
+def other3():
+    create_single_instance_mutex('test')
+    os.mkdir('ready')
+    time.sleep(30)
+
+
 def find_tests():
     return unittest.defaultTestLoader.loadTestsFromTestCase(IPCLockTest)