[Zope-CMF] collector 198 and missing skins

Florent Guillaume fg at nuxeo.com
Fri Feb 18 14:18:38 EST 2005


Florent Guillaume  <fg at nuxeo.com> wrote:
> This bug about _v_skindata reverting to None bit us again and again.
> http://zope.org/Collectors/CMF/198
> I'll have to fix it. Anyone has suggestion on how to best store this? 
> Module scope var (will that work) ? Thread-local var (how) ?

Storing in REQUEST doesn't work as you have no way to get to it from
__getattr__. Here's a work in progress patch that works well for me. I
introduced one of the skin optimizations (caching resolved values) that
SpeedPack does, I'll do the other later.

Comments ? I propose to put this in the 1.5 branch and HEAD.

Florent


--- Skinnable.py.~1.10.6.3.~	Sun Sep  5 01:23:37 2004
+++ Skinnable.py	Fri Feb 18 01:10:31 2005
@@ -18,11 +18,13 @@
 $Id: Skinnable.py,v 1.10.6.3 2004/04/23 21:11:33 sidnei Exp $
 """
 
+#from zLOG import LOG, DEBUG
 import Globals
 from OFS.ObjectManager import ObjectManager
 from Acquisition import ImplicitAcquisitionWrapper, aq_base, aq_inner
 from ExtensionClass import Base
 from AccessControl import ClassSecurityInfo
+from thread import get_ident
 
 
 # superGetAttr is assigned to whatever ObjectManager.__getattr__
@@ -38,9 +40,23 @@
 _marker = []  # Create a new marker object.
 
 
-class SkinnableObjectManager (ObjectManager):
+SKINDATA = {} # mapping thread-id -> (skinobj, ignore, resolve)
+
+class SkinDataCleanup:
+    """Cleanup at the end of the request."""
+    def __init__(self, tid):
+        self.tid = tid
+    def __del__(self):
+        tid = self.tid
+        #LOG('SkinDataCleanup', DEBUG, 'Cleaning up skindata for %s' % tid)
+        if SKINDATA.has_key(tid):
+            del SKINDATA[tid]
+        else:
+            #LOG('SkinDataCleanup', DEBUG, 'MISSING %s' % tid)
+            pass
+
 
-    _v_skindata = None
+class SkinnableObjectManager (ObjectManager):
 
     security = ClassSecurityInfo()
 
@@ -57,16 +73,20 @@
         This should be fast, flexible, and predictable.
         '''
         if not name.startswith('_') and not name.startswith('aq_'):
-            sd = self._v_skindata
+            sd = SKINDATA.get(get_ident())
             if sd is not None:
-                request, ob, ignore = sd
+                ob, ignore, resolve = sd
                 if not ignore.has_key(name):
+                    if resolve.has_key(name):
+                        return resolve[name]
                     subob = getattr(ob, name, _marker)
                     if subob is not _marker:
                         # Return it in context of self, forgetting
                         # its location and acting as if it were located
                         # in self.
-                        return aq_base(subob)
+                        retval = aq_base(subob)
+                        resolve[name] = retval
+                        return retval
                     else:
                         ignore[name] = 1
         if superGetAttr is None:
@@ -108,15 +128,19 @@
         Can be called manually, allowing the user to change
         skins in the middle of a request.
         '''
-        self._v_skindata = None
         skinobj = self.getSkin(skinname)
         if skinobj is not None:
-            self._v_skindata = (self.REQUEST, skinobj, {})
+            tid = get_ident()
+            #LOG('changeSkin', DEBUG, 'Setting up skindata for %s' % tid)
+            SKINDATA[tid] = (skinobj, {}, {})
+            REQUEST = getattr(self, 'REQUEST', None)
+            if REQUEST is not None:
+                REQUEST._hold(SkinDataCleanup(tid))
 
     security.declarePublic('setupCurrentSkin')
     def setupCurrentSkin(self, REQUEST=None):
         '''
-        Sets up _v_skindata so that __getattr__ can find it.
+        Sets up skindata so that __getattr__ can find it.
 
         Can NOT be called manually to change skins in the middle of a 
         request! Use changeSkin for that.
@@ -127,7 +151,7 @@
             # self is not fully wrapped at the moment.  Don't
             # change anything.
             return
-        if self._v_skindata is not None and self._v_skindata[0] is REQUEST:
+        if SKINDATA.has_key(get_ident()):
             # Already set up for this request.
             return
         skinname = self.getSkinNameFromRequest(REQUEST)
@@ -159,18 +183,21 @@
         '''
         superCheckId = SkinnableObjectManager.inheritedAttribute('_checkId')
         if not allow_dup:
-            # Temporarily disable _v_skindata.
+            # Temporarily disable skindata.
             # Note that this depends heavily on Zope's current thread
             # behavior.
-            sd = self._v_skindata
-            self._v_skindata = None
+            tid = get_ident()
+            sd = SKINDATA.get(tid)
+            if sd is not None:
+                del SKINDATA[tid]
             try:
                 base = getattr(self,  'aq_base', self)
                 if not hasattr(base, id):
                     # Cause _checkId to not check for duplication.
                     return superCheckId(self, id, allow_dup=1)
             finally:
-                self._v_skindata = sd
+                if sd is not None:
+                    SKINDATA[tid] = sd
         return superCheckId(self, id, allow_dup)
 
 Globals.InitializeClass(SkinnableObjectManager)






-- 
Florent Guillaume, Nuxeo (Paris, France)   CTO, Director of R&D
+33 1 40 33 71 59   http://nuxeo.com   fg at nuxeo.com


More information about the Zope-CMF mailing list