changeset 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 e083ea8c934e
children 93ecff3f806f
files MoinMoin/wikisync.py MoinMoin/wikiutil.py docs/CHANGES.aschremmer
diffstat 3 files changed, 60 insertions(+), 60 deletions(-) [+]
line wrap: on
line diff
--- a/MoinMoin/wikisync.py	Mon Aug 14 22:29:44 2006 +0200
+++ b/MoinMoin/wikisync.py	Mon Aug 14 22:52:14 2006 +0200
@@ -368,51 +368,58 @@
         
         @param page: a Page object where the tags should be related to
         """
-        
+
         self.page = page
         self.filename = page.getPagePath('synctags', use_underlay=0, check_create=1, isfile=1)
         lock_dir = os.path.join(page.getPagePath('cache', use_underlay=0, check_create=1), '__taglock__')
         self.rlock = lock.ReadLock(lock_dir, 60.0)
         self.wlock = lock.WriteLock(lock_dir, 60.0)
-        self.load()
 
-    def load(self):
-        """ Loads the tags from the data file. """
         if not self.rlock.acquire(3.0):
             raise EnvironmentError("Could not lock in PickleTagStore")
         try:
-            try:
-                datafile = file(self.filename, "rb")
-            except IOError:
-                self.tags = []
-            else:
-                self.tags = pickle.load(datafile)
-                datafile.close()
+            self.load()
         finally:
             self.rlock.release()
+
+    def load(self):
+        """ Loads the tags from the data file. """
+        try:
+            datafile = file(self.filename, "rb")
+        except IOError:
+            self.tags = []
+        else:
+            self.tags = pickle.load(datafile)
+            datafile.close()
     
     def commit(self):
         """ Writes the memory contents to the data file. """
+        datafile = file(self.filename, "wb")
+        pickle.dump(self.tags, datafile, protocol=pickle.HIGHEST_PROTOCOL)
+        datafile.close()
+
+    # public methods ---------------------------------------------------
+    def add(self, **kwargs):
         if not self.wlock.acquire(3.0):
             raise EnvironmentError("Could not lock in PickleTagStore")
         try:
-            datafile = file(self.filename, "wb")
-            pickle.dump(self.tags, datafile, protocol=pickle.HIGHEST_PROTOCOL)
-            datafile.close()
+            self.load()
+            self.tags.append(Tag(**kwargs))
+            self.commit()
         finally:
             self.wlock.release()
 
-    # public methods ---------------------------------------------------
-    def add(self, **kwargs):
-        self.tags.append(Tag(**kwargs))
-        self.commit()
-    
     def get_all_tags(self):
         return self.tags
 
     def clear(self):
         self.tags = []
-        self.commit()
+        if not self.wlock.acquire(3.0):
+            raise EnvironmentError("Could not lock in PickleTagStore")
+        try:
+            self.commit()
+        finally:
+            self.wlock.release()
 
     def fetch(self, iwid_full, direction=None):
         iwid_full = unpackLine(iwid_full)
--- 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()
 
 
 #############################################################################
--- a/docs/CHANGES.aschremmer	Mon Aug 14 22:29:44 2006 +0200
+++ b/docs/CHANGES.aschremmer	Mon Aug 14 22:52:14 2006 +0200
@@ -54,6 +54,7 @@
     * Fixed the MetaDict code to use locks.
     * Fixed bug in request.py that avoided showing a traceback if there was a fault
       after the first headers were sent.
+    * Fixed severe race conditions in the meta dict and the sync tags code.
 
   Other Changes:
     * Refactored conflict resolution and XMLRPC code.