[Checkins] SVN: z3c.saconfig/trunk/src/z3c/saconfig/ Make sure engine factories never unnecisarily re-create engines.

[Brian Sutherland]

Log message for revision 87636:
  Make sure engine factories never unnecisarily re-create engines.

Changed:
  U   z3c.saconfig/trunk/src/z3c/saconfig/README.txt
  U   z3c.saconfig/trunk/src/z3c/saconfig/interfaces.py
  U   z3c.saconfig/trunk/src/z3c/saconfig/utility.py

-=-
Modified: z3c.saconfig/trunk/src/z3c/saconfig/README.txt
===================================================================
--- z3c.saconfig/trunk/src/z3c/saconfig/README.txt	2008-06-21 17:34:20 UTC (rev 87635)
+++ z3c.saconfig/trunk/src/z3c/saconfig/README.txt	2008-06-21 19:13:34 UTC (rev 87636)
@@ -265,3 +265,40 @@
   1
   >>> users[0].name
   u'bob'
+
+Engines and Threading
+=====================
+
+  >>> engine = None
+  >>> def setEngine():
+  ...     global engine
+  ...     engine = engine_factory1()
+
+Engine factories must produce the same engine:
+ 
+  >>> setEngine()
+  >>> engine is engine_factory1()
+  True
+
+Even if you call it in a different thread:
+
+  >>> import threading
+  >>> engine = None
+  >>> t = threading.Thread(target=setEngine)
+  >>> t.start()
+  >>> t.join()
+
+  >>> engine is engine_factory1()
+  True
+
+Unless they are reset:
+  
+  >>> engine_factory1.reset()
+  >>> engine is engine_factory1()
+  False
+
+Even engine factories with the same parameters created at (almost) the same
+time should produce different engines:
+
+  >>> EngineFactory(TEST_DSN1)() is EngineFactory(TEST_DSN1)()
+  False

Modified: z3c.saconfig/trunk/src/z3c/saconfig/interfaces.py
===================================================================
--- z3c.saconfig/trunk/src/z3c/saconfig/interfaces.py	2008-06-21 17:34:20 UTC (rev 87635)
+++ z3c.saconfig/trunk/src/z3c/saconfig/interfaces.py	2008-06-21 19:13:34 UTC (rev 87636)
@@ -56,12 +56,3 @@
 
         This causes the engine to be recreated on next use.
         """
-
-    def getCached():
-        """Return the cached engine.
-        """
-
-    def cache(engine):
-        """Cache the engine.
-        """
-

Modified: z3c.saconfig/trunk/src/z3c/saconfig/utility.py
===================================================================
--- z3c.saconfig/trunk/src/z3c/saconfig/utility.py	2008-06-21 17:34:20 UTC (rev 87635)
+++ z3c.saconfig/trunk/src/z3c/saconfig/utility.py	2008-06-21 19:13:34 UTC (rev 87636)
@@ -2,7 +2,9 @@
 Some reusable, standard implementations of IScopedSession.
 """
 
+import time
 import thread
+import threading
 import sqlalchemy
 
 from zope.interface import implements
@@ -92,6 +94,13 @@
     def siteScopeFunc(self):
         raise NotImplementedError
 
+# Credits: This method of storing engines lifted from zope.app.cache.ram
+_COUNTER = 0
+_COUNTER_LOCK = threading.Lock()
+
+_ENGINES = {}
+_ENGINES_LOCK = threading.Lock()
+
 class EngineFactory(object):
     """An engine factory.
 
@@ -114,16 +123,33 @@
             kw['convert_unicode'] = True
         self._args = args
         self._kw = kw
-        
+        self._key = self._getKey()
+
+    def _getKey(self):
+        """Get a unique key"""
+        global _COUNTER
+        _COUNTER_LOCK.acquire()
+        try:
+            _COUNTER += 1
+            return  "%s_%f_%d" % (id(self), time.time(), _COUNTER)
+        finally:
+            _COUNTER_LOCK.release()
+    
     def __call__(self):
-        engine = self.getCached()
+        # optimistically try get without lock
+        engine = _ENGINES.get(self._key, None)
         if engine is not None:
             return engine
-        # no engine yet, so create a new one
-        args, kw = self.configuration()
-        engine = sqlalchemy.create_engine(*args, **kw)
-        self.cache(engine)
-        return engine
+        # no engine, lock and redo
+        _ENGINES_LOCK.acquire()
+        try:
+            # need to check, another thread may have got there first
+            if self._key not in _ENGINES:
+                args, kw = self.configuration()
+                _ENGINES[self._key] = sqlalchemy.create_engine(*args, **kw)
+            return _ENGINES[self._key]
+        finally:
+            _ENGINES_LOCK.release()
 
     def configuration(self):
         """Returns engine parameters.
@@ -134,17 +160,12 @@
         return self._args, self._kw
 
     def reset(self):
-        engine = self.getCached()
-        if engine is None:
-            return
-        # XXX is disposing the right thing to do?
-        engine.dispose()
-        self.cache(None)
-
-    # XXX what happens if EngineFactory were to be evicted from the ZODB
-    # cache?
-    def getCached(self):
-        return getattr(self, '_v_engine', None)
-
-    def cache(self, engine):
-        self._v_engine = engine
+        _ENGINES_LOCK.acquire()
+        try:
+            if self._key not in _ENGINES:
+                return
+            # XXX is disposing the right thing to do?
+            _ENGINES[self._key].dispose()
+            del _ENGINES[self._key]
+        finally:
+            _ENGINES_LOCK.release()