diff MoinMoin/wikiutil.py @ 1295:9608758dca9a

Fixed severe race conditions in the sync tags and the meta dict code. Before, multiple processes could destroy each other data by keeping two meta dicts instantiated and writing to them.
author Alexander Schremmer <alex AT alexanderweb DOT de>
date Mon, 14 Aug 2006 22:52:14 +0200
parents a7f8dceb4410
children 76a76def8687
line wrap: on
line diff
--- a/MoinMoin/wikiutil.py	Mon Aug 14 22:29:44 2006 +0200
+++ b/MoinMoin/wikiutil.py	Mon Aug 14 22:52:14 2006 +0200
@@ -408,18 +408,23 @@
 
 class MetaDict(dict):
     """ store meta informations as a dict.
-    XXX It is not thread-safe, add locks!
     """
     def __init__(self, metafilename, cache_directory):
         """ create a MetaDict from metafilename """
         dict.__init__(self)
         self.metafilename = metafilename
         self.dirty = False
-        self.loaded = False
         lock_dir = os.path.join(cache_directory, '__metalock__')
         self.rlock = lock.ReadLock(lock_dir, 60.0)
         self.wlock = lock.WriteLock(lock_dir, 60.0)
 
+        if not self.rlock.acquire(3.0):
+            raise EnvironmentError("Could not lock in MetaDict")
+        try:
+            self._get_meta()
+        finally:
+            self.rlock.release()
+
     def _get_meta(self):
         """ get the meta dict from an arbitrary filename.
             does not keep state, does uncached, direct disk access.
@@ -428,14 +433,9 @@
         """
 
         try:
-            if not self.rlock.acquire(3.0):
-                raise EnvironmentError("Could not lock in MetaDict")
-            try:
-                metafile = codecs.open(self.metafilename, "r", "utf-8")
-                meta = metafile.read() # this is much faster than the file's line-by-line iterator
-                metafile.close()
-            finally:
-                self.rlock.release()
+            metafile = codecs.open(self.metafilename, "r", "utf-8")
+            meta = metafile.read() # this is much faster than the file's line-by-line iterator
+            metafile.close()
         except IOError:
             meta = u''
         for line in meta.splitlines():
@@ -444,7 +444,6 @@
             if key in INTEGER_METAS:
                 value = int(value)
             dict.__setitem__(self, key, value)
-        self.loaded = True
 
     def _put_meta(self):
         """ put the meta dict into an arbitrary filename.
@@ -459,44 +458,37 @@
             meta.append("%s: %s" % (key, value))
         meta = '\r\n'.join(meta)
 
-        if not self.wlock.acquire(5.0):
-            raise EnvironmentError("Could not lock in MetaDict")
-        try:
-            metafile = codecs.open(self.metafilename, "w", "utf-8")
-            metafile.write(meta)
-            metafile.close()
-        finally:
-            self.wlock.release()
+        metafile = codecs.open(self.metafilename, "w", "utf-8")
+        metafile.write(meta)
+        metafile.close()
         filesys.chmod(self.metafilename, 0666 & config.umask)
         self.dirty = False
 
     def sync(self, mtime_usecs=None):
-        """ sync the in-memory dict to the persistent store (if dirty) """
-        if self.dirty:
-            if not mtime_usecs is None:
-                self.__setitem__('mtime', str(mtime_usecs))
-            self._put_meta()
+        """ No-Op except for that parameter """
+        if not mtime_usecs is None:
+            self.__setitem__('mtime', str(mtime_usecs))
+        # otherwise no-op
 
     def __getitem__(self, key):
-        try:
-            return dict.__getitem__(self, key)
-        except KeyError:
-            if not self.loaded:
-                self._get_meta() # lazy loading of metadata
-                return dict.__getitem__(self, key)
-            else:
-                raise
+        """ We don't care for cache coherency here. """
+        return dict.__getitem__(self, key)
 
     def __setitem__(self, key, value):
-        """ Sets a dictionary entry. You actually have to call sync to write it
-            to the persistent store. """
+        """ Sets a dictionary entry. """
+        if not self.wlock.acquire(5.0):
+            raise EnvironmentError("Could not lock in MetaDict")
         try:
-            oldvalue = dict.__getitem__(self, key)
-        except KeyError:
-            oldvalue = None
-        if value != oldvalue:
-            dict.__setitem__(self, key, value)
-            self.dirty = True
+            self._get_meta() # refresh cache
+            try:
+                oldvalue = dict.__getitem__(self, key)
+            except KeyError:
+                oldvalue = None
+            if value != oldvalue:
+                dict.__setitem__(self, key, value)
+                self._put_meta() # sync cache
+        finally:
+            self.wlock.release()
 
 
 #############################################################################