changeset 1297:2625857eabf0

Merge with main
author Alexander Schremmer <alex AT alexanderweb DOT de>
date Tue, 15 Aug 2006 22:37:09 +0200
parents 93ecff3f806f (diff) 655978c9f6c3 (current diff)
children d1a4083fc36e
files
diffstat 4 files changed, 64 insertions(+), 62 deletions(-) [+]
line wrap: on
line diff
--- a/MoinMoin/action/SyncPages.py	Tue Aug 15 18:20:39 2006 +0200
+++ b/MoinMoin/action/SyncPages.py	Tue Aug 15 22:37:09 2006 +0200
@@ -24,8 +24,8 @@
 from MoinMoin.PageEditor import PageEditor, conflict_markers
 from MoinMoin.Page import Page
 from MoinMoin.wikidicts import Dict, Group
-from MoinMoin.wikisync import (TagStore, UnsupportedWikiException, SyncPage,
-                               MoinLocalWiki, MoinRemoteWiki, UP, DOWN, BOTH)
+from MoinMoin.wikisync import TagStore, UnsupportedWikiException, SyncPage
+from MoinMoin.wikisync import MoinLocalWiki, MoinRemoteWiki, UP, DOWN, BOTH
 from MoinMoin.util.bdiff import decompress, patch, compress, textdiff
 from MoinMoin.util import diff3
 
--- a/MoinMoin/wikisync.py	Tue Aug 15 18:20:39 2006 +0200
+++ b/MoinMoin/wikisync.py	Tue Aug 15 22:37:09 2006 +0200
@@ -261,6 +261,8 @@
 
     def createSyncPage(self, page_name):
         normalised_name = normalise_pagename(page_name, self.prefix)
+        if not self.request.user.may.write(normalised_name):
+            return None
         if normalised_name is None:
             return None
         return SyncPage(normalised_name, local_rev=Page(self.request, page_name).get_real_rev(), local_name=page_name)
@@ -368,51 +370,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	Tue Aug 15 18:20:39 2006 +0200
+++ b/MoinMoin/wikiutil.py	Tue Aug 15 22:37:09 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	Tue Aug 15 18:20:39 2006 +0200
+++ b/docs/CHANGES.aschremmer	Tue Aug 15 22:37:09 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.