Skip to content

Commit

Permalink
Fix a race condition adding items to the pickle cache; explicitly sup…
Browse files Browse the repository at this point in the history
…port Jython too.
  • Loading branch information
jamadden committed Apr 13, 2015
1 parent 80e0c6e commit 829d155
Show file tree
Hide file tree
Showing 7 changed files with 68 additions and 18 deletions.
4 changes: 3 additions & 1 deletion CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,12 @@

- The Python implementation of ``Persistent`` and ``PickleCache`` now
behave more similarly to the C implementation. In particular, the
Python version can now run the complete ZODB unit test suite.
Python version can now run the complete ZODB and ZEO test suites.

- Fix the hashcode of the Python ``TimeStamp`` on 32-bit platforms.

- Add support for Jython 2.7.


4.0.9 (2015-04-08)
------------------
Expand Down
14 changes: 13 additions & 1 deletion persistent/picklecache.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,14 @@
# Tests may modify this
_SWEEP_NEEDS_GC = not hasattr(sys, 'getrefcount')

# On Jython, we need to explicitly ask it to monitor
# objects if we want a more deterministic GC
if hasattr(gc, 'monitorObject'): #pragma: no cover
_gc_monitor = gc.monitorObject
else:
def _gc_monitor(o):
pass

class RingNode(object):
# 32 byte fixed size wrapper.
__slots__ = ('object', 'next', 'prev')
Expand Down Expand Up @@ -134,7 +142,10 @@ def __setitem__(self, oid, value):

# XXX
if oid in self.persistent_classes or oid in self.data:
if self.data[oid] is not value:
# Have to be careful here, a GC might have just run
# and cleaned up the object
existing_data = self.get(oid)
if existing_data is not None and existing_data is not value:
# Raise the same type of exception as the C impl with the same
# message.
raise ValueError('A different object already has the same oid')
Expand All @@ -153,6 +164,7 @@ def __setitem__(self, oid, value):
self.persistent_classes[oid] = value
else:
self.data[oid] = value
_gc_monitor(value)
self.mru(oid)

def __delitem__(self, oid):
Expand Down
3 changes: 2 additions & 1 deletion persistent/tests/test_persistence.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import sys
py_impl = getattr(platform, 'python_implementation', lambda: None)
_is_pypy3 = py_impl() == 'PyPy' and sys.version_info[0] > 2
_is_jython = py_impl() == 'Jython'

#pylint: disable=R0904,W0212,E1101

Expand Down Expand Up @@ -932,7 +933,7 @@ class Derived(Base):
self.assertEqual(inst.baz, 'bam')
self.assertEqual(inst.qux, 'spam')

if not _is_pypy3:
if not _is_pypy3 and not _is_jython:
def test___setstate___interns_dict_keys(self):
class Derived(self._getTargetClass()):
pass
Expand Down
22 changes: 20 additions & 2 deletions persistent/tests/test_picklecache.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@
#
##############################################################################
import unittest

import gc
_is_jython = hasattr(gc, 'getJythonGCFlags')
_marker = object()

class PickleCacheTests(unittest.TestCase):
Expand Down Expand Up @@ -986,7 +987,22 @@ def test_invalidate_not_in_cache(self):
# Nothing to test, just that it doesn't break
cache._invalidate(p._p_oid)

def test_cache_garbage_collection_bytes_also_deactivates_object(self):
if _is_jython:
def with_deterministic_gc(f):
def test(self):
old_flags = gc.getMonitorGlobal()
gc.setMonitorGlobal(True)
try:
f(self, force_collect=True)
finally:
gc.setMonitorGlobal(old_flags)
return test
else:
def with_deterministic_gc(f):
return f

@with_deterministic_gc
def test_cache_garbage_collection_bytes_also_deactivates_object(self, force_collect=False):
from persistent.interfaces import UPTODATE
from persistent._compat import _b
cache = self._makeOne()
Expand Down Expand Up @@ -1028,6 +1044,8 @@ def test_cache_garbage_collection_bytes_also_deactivates_object(self):

# It also shrank the measured size of the cache;
# this would fail under PyPy if _SWEEP_NEEDS_GC was False
if force_collect:
gc.collect()
self.assertEqual(len(cache), 1)

def test_invalidate_persistent_class_calls_p_invalidate(self):
Expand Down
24 changes: 20 additions & 4 deletions persistent/tests/test_timestamp.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,11 @@
import operator
import unittest

import platform
py_impl = getattr(platform, 'python_implementation', lambda: None)
_is_jython = py_impl() == 'Jython'


class Test__UTC(unittest.TestCase):

def _getTargetClass(self):
Expand Down Expand Up @@ -271,26 +276,37 @@ def test_py_hash_32_64_bit(self):
py = self._makePy(*self.now_ts_args)
self.assertEqual(hash(py), bit_32_hash)


persistent.timestamp.c_long = ctypes.c_int64
# call __hash__ directly to avoid interpreter truncation
# in hash() on 32-bit platforms
self.assertEqual(py.__hash__(), bit_64_hash)
if not _is_jython:
self.assertEqual(py.__hash__(), bit_64_hash)
else:
# Jython 2.7's ctypes module doesn't properly
# implement the 'value' attribute by truncating.
# (It does for native calls, but not visibly to Python).
# Therefore we get back the full python long. The actual
# hash() calls are correct, though, because the JVM uses
# 32-bit ints for its hashCode methods.
self.assertEqual(py.__hash__(), 384009219096809580920179179233996861765753210540033L)
finally:
persistent.timestamp.c_long = orig_c_long

# These are *usually* aliases, but aren't required
# to be
if orig_c_long is ctypes.c_int32:
self.assertEqual(py.__hash__(), bit_32_hash)
elif orig_c_long is ctypes.c_int64:
self.assertEqual(py.__hash__(), bit_64_hash)
else:
self.fail("Unknown bitness")

def test_hash_equal_constants(self):
# The simple constants make it easier to diagnose
# a difference in algorithms
import persistent.timestamp
import ctypes
is_32_bit = persistent.timestamp.c_long == ctypes.c_int32
# We get 32-bit hash values of 32-bit platforms, or on the JVM
is_32_bit = persistent.timestamp.c_long == ctypes.c_int32 or _is_jython

c, py = self._make_C_and_Py(b'\x00\x00\x00\x00\x00\x00\x00\x00')
self.assertEqual(hash(c), 8)
Expand Down
1 change: 1 addition & 0 deletions setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ def _read_file(filename):
'Programming Language :: Python :: 3.4',
"Programming Language :: Python :: Implementation :: CPython",
"Programming Language :: Python :: Implementation :: PyPy",
"Programming Language :: Python :: Implementation :: Jython",
"Framework :: ZODB",
"Topic :: Database",
"Topic :: Software Development :: Libraries :: Python Modules",
Expand Down
18 changes: 9 additions & 9 deletions tox.ini
Original file line number Diff line number Diff line change
@@ -1,18 +1,18 @@
[tox]
envlist =
# Jython support pending 2.7 support, due 2012-07-15 or so. See:
# http://fwierzbicki.blogspot.com/2012/03/adconion-to-fund-jython-27.html
# py26,py27,py32,jython,pypy,coverage,docs
envlist =
# Jython 2.7rc2 does work, but unfortunately has an issue running
# with Tox 1.9.2 (http://bugs.jython.org/issue2325)
# py26,py27,py27-pure,pypy,py32,py33,py34,pypy3,jython,coverage,docs
py26,py27,py27-pure,pypy,py32,py33,py34,pypy3,coverage,docs

[testenv]
deps =
zope.interface
commands =
commands =
python setup.py test -q

[testenv:jython]
commands =
commands =
jython setup.py test -q

[testenv:py27-pure]
Expand All @@ -22,13 +22,13 @@ setenv =
PURE_PYTHON = 1
deps =
{[testenv]deps}
commands =
commands =
python setup.py test -q

[testenv:coverage]
basepython =
python2.6
commands =
commands =
nosetests --with-xunit --with-xcoverage
deps =
zope.interface
Expand All @@ -39,7 +39,7 @@ deps =
[testenv:docs]
basepython =
python2.6
commands =
commands =
sphinx-build -b html -d docs/_build/doctrees docs docs/_build/html
sphinx-build -b doctest -d docs/_build/doctrees docs docs/_build/doctest
deps =
Expand Down

0 comments on commit 829d155

Please sign in to comment.