Handle correctly multiple apr_pool_t * arguments of function in Python bindings to fix negative ref count of _global_py_pool object. * subversion/bindings/swig/include/svn_types.swg (%typemap(default) apr_pool_t *): Retrieve apr_pool_t * pointer from args parameter only when _global_pool is NULL in order to increase correctly ref count. (%typemap(in) apr_pool_t *): Retrieve apr_pool_t * pointer from the Python object and override _global_py_pool when an argument is not the last apr_pool_t * argument. (%typemap(in) apr_pool_t *scratch_pool): New typemap. (%typemap(freearg) apr_pool_t *): Decreament ref count of Python object only once. * subversion/bindings/swig/python/tests/client.py (test_inherited_props): Add tests which TypeError raises when passing non apr_pool_t * object to function which has multiple apr_pool_t * arguments. (test_conflict): Add tests that the _parent_pool attribute of the result wrapper object doesn't point scratch pool object but points result pool object. Patch by: jun66j5 (tewaked by futatuki) Index: subversion/bindings/swig/include/svn_types.swg =================================================================== --- subversion/bindings/swig/include/svn_types.swg (revision 1877963) +++ subversion/bindings/swig/include/svn_types.swg (working copy) @@ -554,23 +554,55 @@ %typemap(default, noblock=1) apr_pool_t *(apr_pool_t *_global_pool = NULL, PyObject *_global_py_pool = NULL) { - if (svn_swig_py_get_pool_arg(args, $descriptor, - &_global_py_pool, &_global_pool)) + if (_global_pool == NULL && + svn_swig_py_get_pool_arg(args, $descriptor, &_global_py_pool, + &_global_pool)) SWIG_fail; $1 = _global_pool; } %typemap(in, noblock=1) apr_pool_t * { - /* Verify that the user supplied a valid pool */ - if ($input != Py_None && $input != _global_py_pool) { - SWIG_Python_TypeError(SWIG_TypePrettyName($descriptor), $input); - SWIG_arg_fail($svn_argnum); - SWIG_fail; - } + if ($input != Py_None && $input != _global_py_pool) + { + apr_pool_t *ptr; + if (svn_swig_py_convert_ptr($input, (void **)&ptr, $descriptor) == 0) + { + $1 = ptr; + } + else + { + SWIG_Python_TypeError(SWIG_TypePrettyName($descriptor), $input); + SWIG_arg_fail($svn_argnum); + SWIG_fail; + } + Py_DECREF(_global_py_pool); + _global_py_pool = $input; + Py_INCREF(_global_py_pool); + } } +/* scratch_pool is always set by "default" typemap, however type check + * is still needed. + */ +%typemap(in, noblock=1) apr_pool_t *scratch_pool { + if ($input != Py_None) + { + apr_pool_t *ptr; + if (svn_swig_py_convert_ptr($input, (void **)&ptr, $descriptor) != 0) + { + SWIG_Python_TypeError(SWIG_TypePrettyName($descriptor), $input); + SWIG_arg_fail($svn_argnum); + SWIG_fail; + } + } +} + %typemap(freearg) apr_pool_t * { - Py_XDECREF(_global_py_pool); + if (_global_py_pool != NULL) + { + Py_XDECREF(_global_py_pool); + _global_py_pool = NULL; + } } #endif Index: subversion/bindings/swig/python/tests/client.py =================================================================== --- subversion/bindings/swig/python/tests/client.py (revision 1877963) +++ subversion/bindings/swig/python/tests/client.py (working copy) @@ -477,6 +477,10 @@ def test_inherited_props(self): """Test inherited props""" + pool = core.Pool() + result_pool = core.Pool(pool) + scratch_pool = core.Pool(pool) + trunk_url = self.repos_uri + b'/trunk' client.propset_remote(b'svn:global-ignores', b'*.q', trunk_url, False, 12, {}, None, self.client_ctx) @@ -483,6 +487,12 @@ head = core.svn_opt_revision_t() head.kind = core.svn_opt_revision_head + self.assertRaises(TypeError, client.propget5, b'svn:global-ignores', + trunk_url, head, head, core.svn_depth_infinity, None, + self.client_ctx, 42) + self.assertRaises(TypeError, client.propget5, b'svn:global-ignores', + trunk_url, head, head, core.svn_depth_infinity, None, + self.client_ctx, None, 42) props, iprops, rev = client.propget5(b'svn:global-ignores', trunk_url, head, head, core.svn_depth_infinity, None, self.client_ctx) @@ -491,7 +501,8 @@ dir1_url = trunk_url + b'/dir1' props, iprops, rev = client.propget5(b'svn:global-ignores', dir1_url, head, head, core.svn_depth_infinity, - None, self.client_ctx) + None, self.client_ctx, result_pool, + scratch_pool) self.assertEqual(iprops[trunk_url], {b'svn:global-ignores':b'*.q\n'}) self.proplist_receiver_trunk_calls = 0 @@ -592,10 +603,15 @@ client.update4((path,), rev, core.svn_depth_unknown, True, False, False, False, False, self.client_ctx) - pool = core.Pool() - conflict = client.conflict_get(trunk_path, self.client_ctx, pool) + # New pools, only for examination where the result comes from. + result_pool = core.Pool() + scratch_pool = core.Pool() + conflict = client.conflict_get(trunk_path, self.client_ctx, + result_pool, scratch_pool) self.assertTrue(isinstance(conflict, client.svn_client_conflict_t)) + self.assertNotEqual(conflict._parent_pool, scratch_pool) + self.assertEqual(conflict._parent_pool, result_pool) conflict_opts = client.conflict_tree_get_resolution_options(conflict, self.client_ctx) @@ -602,8 +618,6 @@ self.assertTrue(isinstance(conflict_opts, list)) self.assert_all_instances_of(conflict_opts, client.svn_client_conflict_option_t) - pool.clear() - @unittest.skip("experimental API, not currently exposed") def test_shelf(self): """Test shelf api."""