Follow-up: Win32/Posix file locking

[Ignacio Vazquez-Abrams]

On Wed, 29 Aug 2001, Sheila King wrote:

> -----------------------(begin MutexFile.py)--------------------------
>  ...
> import os
>
> if os.name == 'nt':
>      from winMutexFile import mutexfile
> elif os.name == 'posix':
>      from posixMutexFile import mutexfile
> else:
>     raise ImportError, "MutexFile is not supported on your platform."
>
>
> class MutexFile(mutexfile):
>     pass
> -----------------------(end MutexFile.py)--------------------------

Nice, clean interface. Can't get much better than this.

> ----------------------(begin posixMutexFile.py)----------------------
>  ...
> import os, fcntl
> from time import time
>
> MAXTIME = 8  # number of secs to retry for a lock before timing out

Eight seconds might be a little long for CGI scripts, but it is writable so
that's fine.

> class mutexfile:
>     def __init__(self, filename):
>         if os.access(filename, os.F_OK):
>             self.filename = filename
>         else:
>             errmssg = filename + \
> 				" does not exist. Can't lock non-existent file."
>             raise IOError, errmssg

The only problem I can see here is if a program expects a certain filename and
someone goes and deletes it. You can't plan for every situation though, so
this risk is acceptable.

>     def __del__(self):
>         try:
>             self.unlock()
>         except:
>             pass
>         try:
>             self.f.close()
>         except:
>             pass
>         if hasattr(self, 'fd'):
>             del self.fd

Once mutexfile is deleted, there are no longer any references to fd, so the
del can be dropped.

>     def getReadLock(self):
>         start_time = time()
>         while time() - start_time < MAXTIME:
>             try:
>                 self.f = open(self.filename, 'r')
>                 self.fd = self.f.fileno()
>                 fcntl.lockf(self.fd, fcntl.LOCK_SH | fcntl.LOCK_NB)
>                 return 1
>             except:
>                 self.f.close()
>                 del self.fd

Ouch. I would put a short delay in here so that CPU usage doesn't go through
the roof by accident.

>         if not hasattr(self, 'fd'):
>             errmssg = self.filename + " temporarily unavailable"
>             raise IOError, errmssg
>
>     def getWriteLock(self):
>         start_time = time()
>         while time() - start_time < MAXTIME:
>             try:
>                 self.f = open(self.filename, 'r+')
>                 self.fd = self.f.fileno()
>                 fcntl.lockf(self.fd, fcntl.LOCK_EX | fcntl.LOCK_NB)
>                 return 1
>             except:
>                 self.f.close()
>                 del self.fd

As above.

>         if not hasattr(self, 'fd'):
>             errmssg = self.filename + " temporarily unavailable"
>             raise IOError, errmssg
>
>     def unlock(self):
>         fcntl.lockf(self.fd, fcntl.LOCK_UN)
>         self.f.close()
>         del self.fd
>
>     def flock(self, flag):
>          ...
>         if flag == 'LOCK_SH':
>             self.getReadLock()
>         elif flag == 'LOCK_EX':
>             self.getWriteLock()
>         elif flag == 'LOCK_UN':
>             self.unlock()
>         else:
>             errmssg = "The flag " + flag + \
> 				" is not implemented for flock"
>             raise NotImplementedError, errmssg
> ----------------------(end posixMutexFile.py)----------------------
>
> ----------------------(begin winMutexFile.py)----------------------
>  ...
> import os
> from time import time
>
> try:
>     import win32file
>     from win32con import GENERIC_READ, GENERIC_WRITE,\
>         FILE_SHARE_READ, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL
> except ImportError, e:
>     print e
>     print "winMutexFile.py requires ActiveState.com Python win32
> extensions"

Hmm. I'm thinking that you need some sort of a raise here so that it stops.

> MAXTIME = 8  # number of secs to retry for a lock before timing out
>
> class mutexfile:
>     def __init__(self, filename):
>         if os.access(filename, os.F_OK):
>             self.filename = filename
>         else:
>             errmssg = filename + \
> 				" does not exist. Can't lock non-existent file."
>             raise IOError, errmssg
>
>     def __del__(self):
>         try:
>             self.unlock()
>         except:
>             pass
>         try:
>             self.f.close()
>         except:
>             pass
>         if hasattr(self, 'fd'):
>             del self.fd
>
>     def getReadLock(self):
>         start_time = time()
>         while time() - start_time < MAXTIME:
>             try:
>                 self.fd = win32file.CreateFile(self.filename,\
> 											  GENERIC_READ,\
>                                               FILE_SHARE_READ, None,\
> 											  OPEN_EXISTING,\
>                                               FILE_ATTRIBUTE_NORMAL, 0)

Looks to me like Agent did a bit of a number on this file...

>                 return 1
>             except:
>                 pass

As in the posixMutexFile module, a delay is probably called for here.

>         if not hasattr(self, 'fd'):
>             errmssg = self.filename + " temporarily unavailable"
>             raise IOError, errmssg
>
>     def getWriteLock(self):
>         start_time = time()
>         while time() - start_time < MAXTIME:
>             try:
>                 self.fd = win32file.CreateFile(self.filename,\
> 											  GENERIC_READ,\
>                                               0, None, OPEN_EXISTING,\
>                                               FILE_ATTRIBUTE_NORMAL, 0)
>                 return 1
>             except:
>                 pass

Another delay here.

>         if not hasattr(self,'fd'):
>             errmssg = self.filename + " temporarily unavailable"
>             raise IOError, errmssg
>
>     def unlock(self):
>         win32file.CloseHandle(self.fd)
>         del self.fd
>
>     def flock(self, flag):
>          ...
>         if flag == 'LOCK_SH':
>             self.getReadLock()
>         elif flag == 'LOCK_EX':
>             self.getWriteLock()
>         elif flag == 'LOCK_UN':
>             self.unlock()
>         else:
>             errmssg = "The flag " + flag + \
> 				" is not implemented for flock"
>             raise NotImplementedError, errmssg
>
> ----------------------(end winMutexFile.py)----------------------

Looks good. Just two other things:

1) I'm wondering if assigning None to fd might be faster than deleting it and
     recreating it.
2) The error messages use string concatenation when using a format string
     might be cleaner.

Other than those few things it looks very good.

-- 
Ignacio Vazquez-Abrams  <ignacio at openservices.net>