Author: jun66j5 Date: Tue May 4 13:53:04 2021 New Revision: 1889487 URL: http://svn.apache.org/viewvc?rev=1889487&view=rev Log: swig-py: Fix doubly destroying memory pool because weakref's callback is not invoked when it is finalized by cyclic garbage collector.
See also https://bugs.python.org/issue40312 for weakref's callbacks during garbage collection. * subversion/bindings/swig/include/proxy_apr.swg (apr_pool_t.valid): Check whether parent pool is valid. * subversion/bindings/swig/python/tests/pool.py (PoolTestCase): Add tests for pools referred from circular reference. Modified: subversion/trunk/subversion/bindings/swig/include/proxy_apr.swg subversion/trunk/subversion/bindings/swig/python/tests/pool.py Modified: subversion/trunk/subversion/bindings/swig/include/proxy_apr.swg URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/swig/include/proxy_apr.swg?rev=1889487&r1=1889486&r2=1889487&view=diff ============================================================================== --- subversion/trunk/subversion/bindings/swig/include/proxy_apr.swg (original) +++ subversion/trunk/subversion/bindings/swig/include/proxy_apr.swg Tue May 4 13:53:04 2021 @@ -142,9 +142,15 @@ struct apr_pool_t { are still valid""" try: self._is_valid - return True except AttributeError: return False + # We must check whether the parent pool is valid even if + # the pool is valid because weakref's callback is not + # invoked when it is finalized by cyclic garbage collector + if self._parent_pool: + return self._parent_pool.valid() + else: + return True def assert_valid(self): """Assert that this memory_pool is still valid.""" Modified: subversion/trunk/subversion/bindings/swig/python/tests/pool.py URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/swig/python/tests/pool.py?rev=1889487&r1=1889486&r2=1889487&view=diff ============================================================================== --- subversion/trunk/subversion/bindings/swig/python/tests/pool.py (original) +++ subversion/trunk/subversion/bindings/swig/python/tests/pool.py Tue May 4 13:53:04 2021 @@ -19,10 +19,11 @@ # # import unittest, weakref, setup_path -import os, tempfile +import os, tempfile, gc import svn.core, svn.client, libsvn.core from svn.core import * from libsvn.core import application_pool, GenericSWIGWrapper +import utils # Test case for the new automatic pool management infrastructure @@ -208,6 +209,51 @@ class PoolTestCase(unittest.TestCase): # We can still destroy and create pools at will svn_pool_destroy(svn_pool_create()) + def _test_pools_in_circular_reference(self, finalizer=False): + + class Circular(object): + + def __init__(self, pool): + self.pool = pool + self.loop = None + + if finalizer: + def __del__(self): + self.pool = self.loop = None + + def create_circularl(): + pool = Pool(libsvn.core.application_pool) + subpool1 = Pool(pool) + subpool2 = Pool(pool) + circularly1 = Circular(pool) + circularly2 = Circular(subpool2) + circularly3 = Circular(subpool1) + circularly1.loop = circularly3 + circularly2.loop = circularly1 + circularly3.loop = circularly2 + refs = weakref.WeakValueDictionary() + refs['pool'] = pool + refs['subpool1'] = subpool1 + refs['subpool2'] = subpool2 + return refs + + refs = create_circularl() + self.assertEqual({'pool', 'subpool1', 'subpool2'}, + set(name for name, pool in refs.items() + if pool is not None)) + gc.collect() + self.assertEqual(set(), set(name for name, pool in refs.items() + if pool is not None)) + + def test_pools_in_circular_reference_without_finalizer(self): + self._test_pools_in_circular_reference(finalizer=False) + + @unittest.skipIf(not utils.IS_PY3, + "Python 2 cannot collect garbage which involves circular " + "references with finalizer") + def test_pools_in_circular_reference_with_finalizer(self): + self._test_pools_in_circular_reference(finalizer=True) + def suite(): return unittest.defaultTestLoader.loadTestsFromTestCase(PoolTestCase)
