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",


Reply via email to