Skip to content

Commit

Permalink
Fix the cleanup of uncommitted blob data under PyPy.
Browse files Browse the repository at this point in the history
  • Loading branch information
jamadden committed Apr 8, 2015
1 parent 255e256 commit b8fbeb9
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 3 deletions.
21 changes: 20 additions & 1 deletion src/ZODB/blob.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,15 @@
# This introduces a threading issue, since a blob file may be destroyed
# via GC in any thread.

# PyPy 2.5 doesn't properly call the cleanup function
# of a weakref when the weakref object dies at the same time
# as the object it refers to. In other words, this doesn't work:
# self._ref = weakref.ref(self, lambda ref: ...)
# because the function never gets called. The Blob class used
# to use that pattern to clean up uncommitted files; now we use this
# module-level global (but still keep a reference in the Blob in case
# we need premature cleanup)
_blob_close_refs = []

@zope.interface.implementer(ZODB.interfaces.IBlob)
class Blob(persistent.Persistent):
Expand All @@ -65,6 +74,7 @@ class Blob(persistent.Persistent):

_p_blob_uncommitted = None # Filename of the uncommitted (dirty) data
_p_blob_committed = None # Filename of the committed data
_p_blob_ref = None # weakreference to self; also in _blob_close_refs

readers = writers = None

Expand Down Expand Up @@ -283,8 +293,13 @@ def _create_uncommitted_file(self):
def cleanup(ref):
if os.path.exists(filename):
os.remove(filename)

try:
_blob_close_refs.remove(ref)
except ValueError:
pass
self._p_blob_ref = weakref.ref(self, cleanup)
_blob_close_refs.append(self._p_blob_ref)

return filename

def _uncommitted(self):
Expand All @@ -293,6 +308,10 @@ def _uncommitted(self):
filename = self._p_blob_uncommitted
if filename is None and self._p_blob_committed is None:
filename = self._create_uncommitted_file()
try:
_blob_close_refs.remove(self._p_blob_ref)
except ValueError:
pass
self._p_blob_uncommitted = self._p_blob_ref = None
return filename

Expand Down
4 changes: 2 additions & 2 deletions src/ZODB/tests/blob_transaction.txt
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ Aborting a blob add leaves the blob unchanged:
... fp.read()
'this is blob 1'

It doesn't clear the file because there is no previously committed version:
It doesn't clear the file because there is no previously committed version:

>>> fname = blob1._p_blob_uncommitted
>>> import os
Expand Down Expand Up @@ -79,7 +79,7 @@ resulting filehandle is accomplished via the filehandle's read method::
>>> blob1afh1.read()
'this is blob 1'

Let's make another filehandle for read only to blob1a. Aach file
Let's make another filehandle for read only to blob1a. Each file
handle has a reference to the (same) underlying blob::

>>> blob1afh2 = blob1a.open("r")
Expand Down
12 changes: 12 additions & 0 deletions src/ZODB/tests/testblob.py
Original file line number Diff line number Diff line change
Expand Up @@ -345,6 +345,18 @@ def gc_blob_removes_uncommitted_data():
>>> os.path.exists(fname)
True
>>> file = blob = None
PyPy not being reference counted actually needs GC to be
explicitly requested. In experiments, it finds the weakref
on the first collection, but only does the cleanup on the second
collection:
>>> import gc
>>> _ = gc.collect()
>>> _ = gc.collect()
Now the file is gone on all platforms:
>>> os.path.exists(fname)
False
"""
Expand Down

0 comments on commit b8fbeb9

Please sign in to comment.