Refactored file locking support:

- Found and added a test for a race condition arising from creating
  and removing lock files.

- Change to leave lock files behind.

- Use the windows locking implementation from msvcrt from the standard
  library, rather than using a custom extension module.

src/ZODB/lock_file.py

@@ -17,31 +17,49 @@ import errno
 import logging
 logger = logging.getLogger("ZODB.lock_file")
 
+class LockError(Exception):
+    """Couldn't lock a file
+    """
+
 try:
     import fcntl
 except ImportError:
     try:
-        from winlock import LockFile as _LockFile
-        from winlock import UnlockFile as _UnlockFile
+        import msvcrt
     except ImportError:
-        def lock_file(file):
-            logger.info('No file-locking support on this platform')
+        def _lock_file(file):
+            raise TypeError('No file-locking support on this platform')
+        def _unlock_file(file):
+            raise TypeError('No file-locking support on this platform')
 
-    # Windows
-    def lock_file(file):
-        # Lock just the first byte
-        _LockFile(file.fileno(), 0, 0, 1, 0)
+    else:
+        # Windows
+        def _lock_file(file):
+            # Lock just the first byte
+            try:
+                msvcrt.locking(file.fileno(), msvcrt.LK_NBLCK, 1)
+            except IOError:
+                raise LockError("Couldn't lock %r" % file.name)
 
-    def unlock_file(file):
-        _UnlockFile(file.fileno(), 0, 0, 1, 0)
+        def _unlock_file(file):
+            try:
+                file.seek(0)
+                msvcrt.locking(file.fileno(), msvcrt.LK_UNLCK, 1)
+            except IOError:
+                raise LockError("Couldn't unlock %r" % file.name)
+                
 else:
     # Unix
     _flags = fcntl.LOCK_EX | fcntl.LOCK_NB
 
-    def lock_file(file):
-        fcntl.flock(file.fileno(), _flags)
+    def _lock_file(file):
+        try:
+            fcntl.flock(file.fileno(), _flags)
+        except IOError:
+            raise LockError("Couldn't lock %r" % file.name)
+            
 
-    def unlock_file(file):
+    def _unlock_file(file):
         # File is automatically unlocked on close
         pass
 
@@ -51,25 +69,29 @@ else:
 # Creating the instance acquires the lock.  The file remains open.  Calling
 # close both closes and unlocks the lock file.
 class LockFile:
+
+    _fp = None
+
     def __init__(self, path):
         self._path = path
+        fp = open(path, 'w+')
+
         try:
-            self._fp = open(path, 'r+')
-        except IOError, e:
-            if e.errno <> errno.ENOENT: raise
-            self._fp = open(path, 'w+')
-        # Acquire the lock and piss on the hydrant
-        try:
-            lock_file(self._fp)
+            _lock_file(fp)
         except:
-            logger.exception("Error locking file %s", path)
+            fp.seek(1)
+            pid = fp.read().strip()[:20]
+            fp.close()
+            logger.exception("Error locking file", path, pid)
             raise
-        print >> self._fp, os.getpid()
-        self._fp.flush()
+
+        self._fp = fp
+        fp.write(" %s\n" % os.getpid())
+        fp.truncate()
+        fp.flush()
 
     def close(self):
         if self._fp is not None:
-            unlock_file(self._fp)
+            _unlock_file(self._fp)
             self._fp.close()
-            os.unlink(self._path)
             self._fp = None

src/ZODB/lock_file.txt

+Lock file support
+=================
+
+The ZODB lock_file module provides support for creating file system
+locks.  These are locks that are implemented with lock files and
+OS-provided locking facilities.  To create a lock, instantiate a
+LockFile object with a file name:
+
+    >>> import ZODB.lock_file
+    >>> lock = ZODB.lock_file.LockFile('lock')
+
+If we try to lock the same name, we'll get a lock error:
+
+    >>> try:
+    ...     ZODB.lock_file.LockFile('lock')
+    ... except ZODB.lock_file.LockError:
+    ...     print "Can't lock file"
+    Can't lock file
+
+To release the lock, use it's close method:
+
+    >>> lock.close()
+
+The lock file is not removed.  It is left behind:
+
+    >>> import os
+    >>> os.path.exists('lock')
+    True
+
+Of course, now that we've released the lock, we can created it again:
+
+    >>> lock = ZODB.lock_file.LockFile('lock')
+    >>> lock.close()

src/ZODB/tests/test_lock_file.py

+##############################################################################
+#
+# Copyright (c) 2004 Zope Corporation and Contributors.
+# All Rights Reserved.
+#
+# This software is subject to the provisions of the Zope Public License,
+# Version 2.0 (ZPL).  A copy of the ZPL should accompany this distribution.
+# THIS SOFTWARE IS PROVIDED "AS IS" AND ANY AND ALL EXPRESS OR IMPLIED
+# WARRANTIES ARE DISCLAIMED, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
+# WARRANTIES OF TITLE, MERCHANTABILITY, AGAINST INFRINGEMENT, AND FITNESS
+# FOR A PARTICULAR PURPOSE.
+#
+##############################################################################
+import os, sys, unittest
+from zope.testing import doctest
+
+import ZODB.lock_file, time, threading
+    
+
+def inc():
+    while 1:
+        try:
+            lock = ZODB.lock_file.LockFile('f.lock')
+        except ZODB.lock_file.LockError:
+            continue
+        else:
+            break
+    f = open('f', 'r+b')
+    v = int(f.readline().strip())
+    time.sleep(0.01)
+    v += 1
+    f.seek(0)
+    f.write('%d\n' % v)
+    f.close()
+    lock.close()
+
+def many_threads_read_and_write():
+    r"""
+    >>> open('f', 'w+b').write('0\n')
+    >>> open('f.lock', 'w+b').write('0\n')
+
+    >>> n = 50
+    >>> threads = [threading.Thread(target=inc) for i in range(n)]
+    >>> _ = [thread.start() for thread in threads]
+    >>> _ = [thread.join() for thread in threads]
+    >>> saved = int(open('f', 'rb').readline().strip())
+    >>> saved == n
+    True
+
+    >>> os.remove('f')
+    >>> os.remove('f.lock')
+    """
+
+def test_suite():
+    suite = unittest.TestSuite()
+    suite.addTest(doctest.DocFileSuite(os.path.join('..', 'lock_file.txt')))
+    suite.addTest(doctest.DocTestSuite())
+    return suite