Author: svn-role Date: Sat Jan 20 04:00:04 2024 New Revision: 1915338 URL: http://svn.apache.org/viewvc?rev=1915338&view=rev Log: Merge r1915316 from trunk:
* r1915316 swig-py: Fix `none_dealloc` error caused by reference count issue in `apply_textdelta` invoked from `svn.repos.replay()`. Justification: Fix Python interpreter crash which often occurs with large repositories. Votes: +1: jun66j5, futatuki Added: subversion/branches/1.14.x/subversion/bindings/swig/python/tests/data/repository-deltas.dump - copied unchanged from r1915316, subversion/trunk/subversion/bindings/swig/python/tests/data/repository-deltas.dump Modified: subversion/branches/1.14.x/ (props changed) subversion/branches/1.14.x/STATUS subversion/branches/1.14.x/subversion/bindings/swig/python/libsvn_swig_py/swigutil_py.c subversion/branches/1.14.x/subversion/bindings/swig/python/tests/repository.py Propchange: subversion/branches/1.14.x/ ------------------------------------------------------------------------------ Merged /subversion/trunk:r1915316 Modified: subversion/branches/1.14.x/STATUS URL: http://svn.apache.org/viewvc/subversion/branches/1.14.x/STATUS?rev=1915338&r1=1915337&r2=1915338&view=diff ============================================================================== --- subversion/branches/1.14.x/STATUS (original) +++ subversion/branches/1.14.x/STATUS Sat Jan 20 04:00:04 2024 @@ -43,11 +43,3 @@ Veto-blocked changes: Approved changes: ================= - - * r1915316 - swig-py: Fix `none_dealloc` error caused by reference count issue in - `apply_textdelta` invoked from `svn.repos.replay()`. - Justification: - Fix Python interpreter crash which often occurs with large repositories. - Votes: - +1: jun66j5, futatuki Modified: subversion/branches/1.14.x/subversion/bindings/swig/python/libsvn_swig_py/swigutil_py.c URL: http://svn.apache.org/viewvc/subversion/branches/1.14.x/subversion/bindings/swig/python/libsvn_swig_py/swigutil_py.c?rev=1915338&r1=1915337&r2=1915338&view=diff ============================================================================== --- subversion/branches/1.14.x/subversion/bindings/swig/python/libsvn_swig_py/swigutil_py.c (original) +++ subversion/branches/1.14.x/subversion/bindings/swig/python/libsvn_swig_py/swigutil_py.c Sat Jan 20 04:00:04 2024 @@ -2321,8 +2321,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; } @@ -2839,8 +2837,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(); @@ -2863,13 +2860,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. */ @@ -2890,6 +2886,7 @@ parse_fn3_apply_textdelta(svn_txdelta_wi err = SVN_NO_ERROR; finished: + Py_XDECREF(result); svn_swig_py_release_py_lock(); return err; } Modified: subversion/branches/1.14.x/subversion/bindings/swig/python/tests/repository.py URL: http://svn.apache.org/viewvc/subversion/branches/1.14.x/subversion/bindings/swig/python/tests/repository.py?rev=1915338&r1=1915337&r2=1915338&view=diff ============================================================================== --- subversion/branches/1.14.x/subversion/bindings/swig/python/tests/repository.py (original) +++ subversion/branches/1.14.x/subversion/bindings/swig/python/tests/repository.py Sat Jan 20 04:00:04 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",