[Zodb-checkins] SVN: ZODB/trunk/src/ZODB/ If there is a failure while FileStorage is finalizing a transaction,

[Jim Fulton]

Log message for revision 91396:
  If there is a failure while FileStorage is finalizing a transaction,
  the file storage is closed because it's internal meta data may be
  invalid.
  

Changed:
  U   ZODB/trunk/src/ZODB/FileStorage/FileStorage.py
  U   ZODB/trunk/src/ZODB/tests/testFileStorage.py

-=-
Modified: ZODB/trunk/src/ZODB/FileStorage/FileStorage.py
===================================================================
--- ZODB/trunk/src/ZODB/FileStorage/FileStorage.py	2008-09-23 12:56:06 UTC (rev 91395)
+++ ZODB/trunk/src/ZODB/FileStorage/FileStorage.py	2008-09-23 14:28:03 UTC (rev 91396)
@@ -657,21 +657,33 @@
             self._lock_release()
 
     def _finish(self, tid, u, d, e):
-        nextpos=self._nextpos
-        if nextpos:
-            file=self._file
-
+        # If self._nextpos is 0, then the transaction didn't write any
+        # data, so we don't bother writing anything to the file.
+        if self._nextpos:
             # Clear the checkpoint flag
-            file.seek(self._pos+16)
-            file.write(self._tstatus)
-            file.flush()
+            self._file.seek(self._pos+16)
+            self._file.write(self._tstatus)
+            try:
+                # At this point, we may have committed the data to disk.
+                # If we fail from here, we're in bad shape.
+                self._finish_finish(tid)
+            except:
+                # Ouch.  This is bad.  Let's try to get back to where we were
+                # and then roll over and die
+                logger.critical("Failure in _finish. Closing.", exc_info=True)
+                self.close()
+                raise
 
-            if fsync is not None: fsync(file.fileno())
+    def _finish_finish(self, tid):
+        # This is a separate method to allow tests to replace it with
+        # something broken. :)
+        
+        self._file.flush()
+        if fsync is not None:
+            fsync(self._file.fileno())
 
-            self._pos = nextpos
-
-            self._index.update(self._tindex)
-
+        self._pos = self._nextpos
+        self._index.update(self._tindex)
         self._ltid = tid
 
     def _abort(self):

Modified: ZODB/trunk/src/ZODB/tests/testFileStorage.py
===================================================================
--- ZODB/trunk/src/ZODB/tests/testFileStorage.py	2008-09-23 12:56:06 UTC (rev 91395)
+++ ZODB/trunk/src/ZODB/tests/testFileStorage.py	2008-09-23 14:28:03 UTC (rev 91396)
@@ -502,6 +502,60 @@
 
     """
 
+def deal_with_finish_failures():
+    r"""
+    
+    It's really bad to get errors in FileStorage's _finish method, as
+    that can cause the file storage to be in an inconsistent
+    state. The data file will be fine, but the internal data
+    structures might be hosed. For this reason, FileStorage will close
+    if there is an error after it has finished writing transaction
+    data.  It bothers to do very little after writing this data, so
+    this should rarely, if ever, happen.
+
+    >>> fs = ZODB.FileStorage.FileStorage('data.fs')
+    >>> db = DB(fs)
+    >>> conn = db.open()
+    >>> conn.root()[1] = 1
+    >>> transaction.commit()
+
+    Now, we'll indentially break the file storage. It provides a hook
+    for this purpose. :)
+
+    >>> fs._finish_finish = lambda : None
+    >>> conn.root()[1] = 1
+
+    >>> import zope.testing.loggingsupport
+    >>> handler = zope.testing.loggingsupport.InstalledHandler(
+    ...     'ZODB.FileStorage')
+    >>> transaction.commit()
+    Traceback (most recent call last):
+    ...
+    TypeError: <lambda>() takes no arguments (1 given)
+
+    
+    >>> print handler
+    ZODB.FileStorage CRITICAL
+      Failure in _finish. Closing.
+
+    >>> handler.uninstall()
+
+    >>> fs.load('\0'*8, '')
+    Traceback (most recent call last):
+    ...
+    ValueError: I/O operation on closed file
+
+    >>> db.close()
+    >>> fs = ZODB.FileStorage.FileStorage('data.fs')
+    >>> db = DB(fs)
+    >>> conn = db.open()
+    >>> conn.root()
+    {1: 1}
+
+    >>> transaction.abort()
+    >>> db.close()
+    """
+
 def test_suite():
     from zope.testing import doctest