Fix possible data corruption after FileStorage is truncated to roll back a transaction

[jmuchemb]

See the commit message for more information.
I wanted to attach the stress test I wrote to reproduce and understand the issue, but GitHub does not want *.py files. It was already sent to the mailing-list so you can find it there:
http://thread.gmane.org/gmane.comp.web.zope.zodb/12685/focus=12751

I'd like to also apply it on 3.10 branch.

/cc @vpelletier

[tseaver]

@jmuchemb Can you please open a separate PR against the 3.10 branch (maybe after responding to review below).

@jimfulton This patch looks reasonable to me, but doesn't contain tests that the pooled files have been flushed. Can you suggest a reasonable strategy for such a test?

[jimfulton]

Well, it should be possible to set up a simple scenario that demonstrates the problem without a stress test:

• Create a database with a small record for object 0.
• Perform a transaction up to the vote call that writes object 1.
• do a read of object 0.
• abort the transaction
• commit a record of the same size but with different data for object 1 in a new transaction.
• read object 1. Presumably, you'll get the old.

If you can reproduce the problem without the patch, then you can try it with the patch, and if that fixes it, you have a test.

One thing that puzzles me is that when reading, file-storage seeks to the record position of the record read, and IIRC seek flushes before seeking, but perhaps I'm mistaken.

[jmuchemb]

I added a unit test, and Travis reports the fix only works for Python 2. I need a Python3 setup of ZODB...

[jimfulton]

Thanks Julien.

[tseaver]

@jmuchemb If you have tox set up, you can just run tox -e py33 inside your checkout.

[jmuchemb]

There's no API in Python 3 to invalidate the read buffer of a file that is open in read-only mode. So we have the choice between:

• reset the FilePool (do flush = empty for Python 3)
• make FilePool.get open files with r+b (in order not to keep FileStorage(read_only=True) working with non-writable files, we'd also need a read_only parameter to FilePool.

[jmuchemb]

I chose the first solution and all tests pass.
You can merge if you agree.

[jmuchemb]

reset the FilePool (do flush = empty for Python 3)

Contrary to empty(), I think flush() also needs to use write_lock() for Python 3, so I amended with the following change:

--- a/src/ZODB/FileStorage/FileStorage.py
+++ b/src/ZODB/FileStorage/FileStorage.py
@@ -2108,7 +2108,9 @@ def flush(self):

     # Unfortunately, Python 3.x has no API to flush read buffers.
     if sys.version_info.major > 2:
-        flush = empty
+        def flush(self):
+            with self.write_lock():
+                self.empty()

     def close(self):
         with self._cond:

I also did another commit to update the changelog.

[jimfulton]

Sadly, flushing isn't good enough on Python 2 and mac. Making a new PR to always empty.

#57

[jmuchemb]

Contrary to Python 3, Python 2 uses file functions from the C standard library (here fflush), so it's possible that the optimization only worked with the glibc.