Author: jun66j5
Date: Fri Jan 19 06:26:36 2024
New Revision: 1915316
URL: http://svn.apache.org/viewvc?rev=1915316&view=rev
Log:
Following up on r1912500, fix `none_dealloc` error caused by `apply_textdelta`
function incorrectly decrementing the reference count of the `None` object
twice when `delta.Editor.apply_textdelta` returns `None`, and the issue that
`parse_fn3_apply_textdelta` function which doesn't decrement the reference
count of the handler returned from `ParseFn3.apply_textdelta`.
* subversion/bindings/swig/python/libsvn_swig_py/swigutil_py.c
(apply_textdelta): Remove one of the calls of `Py_DECREF` for `Py_None`.
(parse_fn3_apply_textdelta): Decrement the reference count of the handler
returned from `ParseFns3.apply_textdelta`.
* subversion/bindings/swig/python/tests/data/repository-deltas.dump:
Add deltas dump file for `parse_fn3_apply_textdelta`.
* subversion/bindings/swig/python/tests/repository.py
(test_delta_editor_apply_textdelta_handler_refcount): Add tests.
(test_parse_fns3_apply_textdelta_handler_refcount): Add tests.
Reported by: https://trac.edgewall.org/ticket/13704
Reviewed by: futatuki
Added:
subversion/trunk/subversion/bindings/swig/python/tests/data/repository-deltas.dump
(with props)
Modified:
subversion/trunk/subversion/bindings/swig/python/libsvn_swig_py/swigutil_py.c
subversion/trunk/subversion/bindings/swig/python/tests/repository.py
Modified:
subversion/trunk/subversion/bindings/swig/python/libsvn_swig_py/swigutil_py.c
URL:
http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/swig/python/libsvn_swig_py/swigutil_py.c?rev=1915316&r1=1915315&r2=1915316&view=diff
==============================================================================
---
subversion/trunk/subversion/bindings/swig/python/libsvn_swig_py/swigutil_py.c
(original)
+++
subversion/trunk/subversion/bindings/swig/python/libsvn_swig_py/swigutil_py.c
Fri Jan 19 06:26:36 2024
@@ -2342,8 +2342,6 @@ static svn_error_t *apply_textdelta(void
in Python. */
if (result == Py_None)
{
- Py_DECREF(result);
-
*handler = svn_delta_noop_window_handler;
*h_baton = NULL;
}
@@ -2860,8 +2858,7 @@ parse_fn3_apply_textdelta(svn_txdelta_wi
{
PyObject *editor = NULL, *baton_item = NULL, *py_pool = NULL;
PyObject *ib = node_baton;
- apr_pool_t *pool;
- PyObject *result;
+ PyObject *result = NULL;
svn_error_t *err;
svn_swig_py_acquire_py_lock();
@@ -2884,13 +2881,12 @@ parse_fn3_apply_textdelta(svn_txdelta_wi
in Python. */
if (result == Py_None)
{
- Py_DECREF(result);
-
*handler = svn_delta_noop_window_handler;
*handler_baton = NULL;
}
else
{
+ apr_pool_t *pool;
/* return the thunk for invoking the handler. the baton creates
new reference of our 'result' reference, which is the handler,
so we release it even if no error. */
@@ -2911,6 +2907,7 @@ parse_fn3_apply_textdelta(svn_txdelta_wi
err = SVN_NO_ERROR;
finished:
+ Py_XDECREF(result);
svn_swig_py_release_py_lock();
return err;
}
Added:
subversion/trunk/subversion/bindings/swig/python/tests/data/repository-deltas.dump
URL:
http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/swig/python/tests/data/repository-deltas.dump?rev=1915316&view=auto
==============================================================================
Binary file - no diff available.
Propchange:
subversion/trunk/subversion/bindings/swig/python/tests/data/repository-deltas.dump
------------------------------------------------------------------------------
svn:mime-type = application/octet-stream
Modified: subversion/trunk/subversion/bindings/swig/python/tests/repository.py
URL:
http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/swig/python/tests/repository.py?rev=1915316&r1=1915315&r2=1915316&view=diff
==============================================================================
--- subversion/trunk/subversion/bindings/swig/python/tests/repository.py
(original)
+++ subversion/trunk/subversion/bindings/swig/python/tests/repository.py Fri
Jan 19 06:26:36 2024
@@ -331,6 +331,33 @@ class SubversionRepositoryTestCase(unitt
finally:
stream.close()
+ def test_parse_fns3_apply_textdelta_handler_refcount(self):
+ handler = lambda node_baton: None
+ handler_ref = weakref.ref(handler)
+
+ class ParseFns3(repos.ParseFns3):
+ def __init__(self, handler):
+ self.called = set()
+ self.handler = handler
+ def apply_textdelta(self, node_baton):
+ self.called.add('apply_textdelta')
+ return self.handler
+
+ dumpfile = os.path.join(os.path.dirname(__file__), 'data',
+ 'repository-deltas.dump')
+ pool = Pool()
+ subpool = Pool(pool)
+ parser = ParseFns3(handler)
+ ptr, baton = repos.make_parse_fns3(parser, subpool)
+ with open(dumpfile, "rb") as stream:
+ repos.parse_dumpstream3(stream, ptr, baton, False, None)
+ del ptr, baton, stream
+
+ self.assertIn('apply_textdelta', parser.called)
+ self.assertNotEqual(None, handler_ref())
+ del parser, handler, subpool, ParseFns3
+ self.assertEqual(None, handler_ref())
+
def test_get_logs(self):
"""Test scope of get_logs callbacks"""
logs = []
@@ -427,6 +454,32 @@ class SubversionRepositoryTestCase(unitt
self.assertEqual(sys.getrefcount(e_ptr), 2,
"leak on editor baton after replay with an error")
+ def test_delta_editor_apply_textdelta_handler_refcount(self):
+ handler = lambda textdelta: None
+ handler_ref = weakref.ref(handler)
+
+ class Editor(delta.Editor):
+ def __init__(self, handler):
+ self.called = set()
+ self.handler = handler
+ def apply_textdelta(self, file_baton, base_checksum, pool=None):
+ self.called.add('apply_textdelta')
+ return self.handler
+
+ pool = Pool()
+ subpool = Pool(pool)
+ root = fs.revision_root(self.fs, 3) # change of trunk/README.txt
+ editor = Editor(handler)
+ e_ptr, e_baton = delta.make_editor(editor, subpool)
+ repos.replay(root, e_ptr, e_baton, subpool)
+ del e_ptr, e_baton
+
+ self.assertIn('apply_textdelta', editor.called)
+ self.assertNotEqual(None, handler_ref())
+ del root, editor, handler, Editor
+ del subpool
+ self.assertEqual(None, handler_ref())
+
def test_retrieve_and_change_rev_prop(self):
"""Test playing with revprops"""
self.assertEqual(repos.fs_revision_prop(self.repos, self.rev, b"svn:log",
