Rearranged the sub-connection closing mechanism. The old method was subject

to database connection leaks when the mount point object was garbage
collected.

src/ZODB/Mount.py

@@ -84,8 +84,8 @@
 ##############################################################################
 """Mounted database support
 
-$Id: Mount.py,v 1.7 2000/11/07 13:03:32 jim Exp $"""
-__version__='$Revision: 1.7 $'[11:-2]
+$Id: Mount.py,v 1.8 2000/12/30 20:04:49 shane Exp $"""
+__version__='$Revision: 1.8 $'[11:-2]
 
 import thread, Persistence, Acquisition
 import ExtensionClass, string, time, sys
@@ -124,7 +124,6 @@ class MountPoint(Persistence.Persistent, Acquisition.Implicit):
     # Default values for non-persistent variables.
     _v_db = None
     _v_data = None
-    _v_close_db = 0
     _v_connect_error = None
 
     def __init__(self, path, params=None, classDefsFromRoot=1):
@@ -193,50 +192,17 @@ class MountPoint(Persistence.Persistent, Acquisition.Implicit):
             dblock.release()
         return db, newMount
 
-    def __repr__(self):
-        return "%s %s" % (self.__class__.__name__, self._path)
+    def _getMountpointId(self):
+        return self.__mountpoint_id
 
-    def _close(self):
-        # The onCloseCallback handler.
-        # Closes a single connection to the database
-        # and possibly the database itself.
-        t = self._v_data
-        if t is not None:
-            self._v_data = None
-            data = t[0]
-            if getattr(data, '_v__object_deleted__', 0):
-                # This mount point has been deleted.
-                del data._v__object_deleted__
-                self._v_close_db = 1
-            if data is not None:
-                conn = data._p_jar
-                if conn is not None:
-                    try: del conn._mount_parent_jar
-                    except: pass
-                    if conn._db is not None:
-                        conn.close()
-        if self._v_close_db:
-            # Stop using this database. Close it if no other
-            # MountPoint is using it.
-            dblock.acquire()
-            try:
-                self._v_close_db = 0
-                self._v_db = None
-                params = self._params
-                if dbs.has_key(params):
-                    dbInfo = dbs[params]
-                    db, mounts = dbInfo
-                    try: del mounts[self.__mountpoint_id]
-                    except: pass
-                    if len(mounts) < 1:
-                        # No more mount points are using this database.
-                        del dbs[params]
-                        db.close()
-                        LOG('ZODB', INFO, 'Closed database: %s' % params)
-            finally:
-                dblock.release()
+    def _getMountParams(self):
+        return self._params
 
-    def __openConnection(self, parent):
+    def __repr__(self):
+        return "%s(%s, %s)" % (self.__class__.__name__, repr(self._path),
+                               self._params)
+
+    def _openMountableConnection(self, parent):
         # Opens a new connection to the database.
         db = self._v_db
         if db is None:
@@ -249,59 +215,60 @@ class MountPoint(Persistence.Persistent, Acquisition.Implicit):
             # Get _p_jar from parent.
             self._p_jar = jar = parent._p_jar
         conn = db.open(version=jar.getVersion())
+
         # Add an attribute to the connection which
         # makes it possible for us to find the primary
         # database connection.  See ClassFactoryForMount().
         conn._mount_parent_jar = jar
 
-        try:
-            jar.onCloseCallback(self._close)
-            obj = self._getMountRoot(conn.root())
-            data = getattr(obj, 'aq_base', obj)
-            # Store the data object in a tuple to hide from acquisition.
-            self._v_data = (data,)
-        except:
-            # Close the connection before processing the exception.
-            del conn._mount_parent_jar
-            conn.close()
-            raise
-        if newMount:
-            id = data.id
-            if callable(id):
-                id = id()
-            p = string.join(parent.getPhysicalPath() + (id,), '/')
-            LOG('ZODB', INFO, 'Mounted database %s at %s' % \
-                (self._params, p))
+        mcc = MountedConnectionCloser(self, conn)
+        jar.onCloseCallback(mcc)
+        return conn, newMount
+
+    def _getObjectFromConnection(self, conn):
+        obj = self._getMountRoot(conn.root())
+        data = getattr(obj, 'aq_base', obj)
+        # Store the data object in a tuple to hide from acquisition.
+        self._v_data = (data,)
         return data
 
-    def __of__(self, parent):
-        # Accesses the database, returning an acquisition
-        # wrapper around the connected object rather than around self.
+    def _getOrOpenObject(self, parent):
         t = self._v_data
         if t is None:
             self._v_connect_error = None
+            conn = None
             try:
-                data = self.__openConnection(parent)
+                conn, newMount = self._openMountableConnection(parent)
+                data = self._getObjectFromConnection(conn)
+                if newMount:
+                    id = data.getId()
+                    p = string.join(parent.getPhysicalPath() + (id,), '/')
+                    LOG('ZODB', INFO, 'Mounted database %s at %s' %
+                        (self._getMountParams(), p))
             except:
-                self._v_close_db = 1
+                # Broken database.
+                if conn is not None:
+                    conn._close_mounted_db = 1
                 self._logConnectException()
-                # Broken database. Wrap around self.
-                return Acquisition.ImplicitAcquisitionWrapper(self, parent)
+                raise
         else:
             data = t[0]
 
         return data.__of__(parent)
+        
+    def __of__(self, parent):
+        # Accesses the database, returning an acquisition
+        # wrapper around the connected object rather than around self.
+        try:
+            return self._getOrOpenObject(parent)
+        except:
+            return Acquisition.ImplicitAcquisitionWrapper(
+                self, parent)
 
     def _test(self, parent):
         '''Tests the database connection.
         '''
-        if self._v_data is None:
-            try:
-                data = self.__openConnection(parent)
-            except:
-                self._v_close_db = 1
-                self._logConnectException()
-                raise
+        self._getOrOpenObject(parent)
         return 1
 
     def _getMountRoot(self, root):
@@ -311,22 +278,84 @@ class MountPoint(Persistence.Persistent, Acquisition.Implicit):
         try:
             app = root['Application']
         except:
-            raise MountedStorageError, \
-                  'No \'Application\' object exists in the mountable database.'
+            raise MountedStorageError, (
+                "No 'Application' object exists in the mountable database.")
         try:
             return app.unrestrictedTraverse(self._path)
         except:
-            raise MountedStorageError, \
-                  ('The path \'%s\' was not found in the mountable database.' \
-                   % self._path)
+            raise MountedStorageError, (
+                "The path '%s' was not found in the mountable database."
+                % self._path)
 
     def _logConnectException(self):
         '''Records info about the exception that just occurred.
         '''
-        from cStringIO import StringIO
+        try:
+            from cStringIO import StringIO
+        except:
+            from StringIO import StringIO
         import traceback
         exc = sys.exc_info()
-        LOG('ZODB', WARNING, 'Failed to mount database. %s (%s)' % exc[:2])
+        LOG('ZODB', WARNING, 'Failed to mount database. %s (%s)' % exc[:2],
+            error=exc)
         f=StringIO()
         traceback.print_tb(exc[2], 100, f)
         self._v_connect_error = (exc[0], exc[1], f.getvalue())
+        exc = None
+
+
+class MountedConnectionCloser:
+    '''Closes the connection used by the mounted database
+    while performing other cleanup.
+    '''
+
+    def __init__(self, mountpoint, conn):
+        # conn is the child connection.
+        self.mp = mountpoint
+        self.conn = conn
+
+    def __call__(self):
+        # The onCloseCallback handler.
+        # Closes a single connection to the database
+        # and possibly the database itself.
+        conn = self.conn
+        close_db = 0
+        if conn is not None:
+            mp = self.mp
+            # Remove potential circular references.
+            self.conn = None
+            self.mp = None
+            # Detect whether we should close the database.
+            close_db = getattr(conn, '_close_mounted_db', 0)
+            t = mp._v_data
+            if t is not None:
+                mp._v_data = None
+                data = t[0]
+                if not close_db and getattr(data, '_v__object_deleted__', 0):
+                    # This mount point has been deleted.
+                    del data._v__object_deleted__
+                    close_db = 1
+            # Close the child connection.
+            try: del conn._mount_parent_jar
+            except: pass
+            conn.close()
+        
+        if close_db:
+            # Stop using this database. Close it if no other
+            # MountPoint is using it.
+            dblock.acquire()
+            try:
+                params = mp._getMountParams()
+                mp._v_db = None
+                if dbs.has_key(params):
+                    dbInfo = dbs[params]
+                    db, mounts = dbInfo
+                    try: del mounts[mp._getMountpointId()]
+                    except: pass
+                    if len(mounts) < 1:
+                        # No more mount points are using this database.
+                        del dbs[params]
+                        db.close()
+                        LOG('ZODB', INFO, 'Closed database: %s' % params)
+            finally:
+                dblock.release()