Author: svn-role
Date: Fri Oct 20 04:00:04 2023
New Revision: 1913135

URL: http://svn.apache.org/viewvc?rev=1913135&view=rev
Log:
Merge the r1912500 group from trunk:

 * r1912500, r1912501, r1912502, r1912503, r1912515, r1912517, r1912691
   swig-py: Use pure Python objects as edit/parse_fns3 and decendant batons. 
   Justification:
     Bug fix. Issue #4916, #4917, #4918
   Votes:
     +1: futatuki
     +0: dsahlberg (not enough experience for +1, but looks good)

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/libsvn_swig_py/swigutil_py.h
    subversion/branches/1.14.x/subversion/bindings/swig/python/svn/delta.py
    subversion/branches/1.14.x/subversion/bindings/swig/python/svn/repos.py
    subversion/branches/1.14.x/subversion/bindings/swig/python/tests/delta.py
    
subversion/branches/1.14.x/subversion/bindings/swig/python/tests/repository.py
    subversion/branches/1.14.x/subversion/bindings/swig/svn_delta.i
    subversion/branches/1.14.x/subversion/bindings/swig/svn_repos.i

Propchange: subversion/branches/1.14.x/
------------------------------------------------------------------------------
  Merged /subversion/trunk:r1912500-1912503,1912515,1912517,1912691

Modified: subversion/branches/1.14.x/STATUS
URL: 
http://svn.apache.org/viewvc/subversion/branches/1.14.x/STATUS?rev=1913135&r1=1913134&r2=1913135&view=diff
==============================================================================
--- subversion/branches/1.14.x/STATUS (original)
+++ subversion/branches/1.14.x/STATUS Fri Oct 20 04:00:04 2023
@@ -41,11 +41,3 @@ Veto-blocked changes:
 
 Approved changes:
 =================
-
- * r1912500, r1912501, r1912502, r1912503, r1912515, r1912517, r1912691
-   swig-py: Use pure Python objects as edit/parse_fns3 and decendant batons. 
-   Justification:
-     Bug fix. Issue #4916, #4917, #4918
-   Votes:
-     +1: futatuki
-     +0: dsahlberg (not enough experience for +1, but looks good)

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=1913135&r1=1913134&r2=1913135&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
 Fri Oct 20 04:00:04 2023
@@ -1762,44 +1762,97 @@ static svn_error_t *type_conversion_erro
 
 /*** Editor Wrapping ***/
 
-/* this baton is used for the editor, directory, and file batons. */
-typedef struct item_baton {
-  PyObject *editor;     /* the editor handling the callbacks */
-  PyObject *baton;      /* the dir/file baton (or NULL for edit baton) */
-  apr_pool_t *pool;     /* top-level pool */
-} item_baton;
-
-static item_baton *make_baton(apr_pool_t *pool,
-                              PyObject *editor,
-                              PyObject *baton)
-{
-  item_baton *newb = apr_palloc(pool, sizeof(*newb));
-
-  /* Note: We steal the caller's reference to 'baton'. */
-  Py_INCREF(editor);
-  newb->editor = editor;
-  newb->baton = baton;
-  newb->pool = pool;
+static PyObject *
+make_baton(apr_pool_t *pool, PyObject *parent, PyObject *baton)
+{
+  PyObject *newb;
 
+  newb = PyObject_CallMethod(parent, "make_decendant", "O&O",
+                             make_ob_pool, pool, baton);
+  /* We always borrow the reference in ancestor's dict during the C API
+     processing, so that we never leak the reference even the API aborted
+     by some error */
+  Py_XDECREF(newb);
   return newb;
 }
 
-static svn_error_t *close_baton(void *baton,
-                                const char *method)
+/* Get 'editor' and 'baton' from _ItemBaton instance. The caller
+   should be within a Python thread lock. */
+static svn_error_t *
+unwrap_item_baton(PyObject **editor, PyObject **baton, PyObject *item_baton)
 {
-  item_baton *ib = baton;
+  svn_error_t *err;
+
+  if ((*editor = PyObject_GetAttrString(item_baton, "editor")) == NULL)
+    {
+      err = callback_exception_error();
+      *baton = NULL;
+      goto finished;
+    }
+  if ((*baton = PyObject_GetAttrString(item_baton, "baton")) == NULL)
+    {
+      Py_CLEAR(*editor);
+      err = callback_exception_error();
+      goto finished;
+    }
+  err = SVN_NO_ERROR;
+ finished:
+  Py_XDECREF(*editor);
+  Py_XDECREF(*baton);
+  return err;
+}
+
+/* Get 'editor', 'baton', 'pool' from _ItemBaton instance. The caller
+   should be within a Python thread lock. */
+static svn_error_t *
+unwrap_item_baton_with_pool(PyObject **editor, PyObject **baton,
+                            PyObject **py_pool, PyObject *item_baton)
+{
+  svn_error_t *err;
+
+  if ((err = unwrap_item_baton(editor, baton, item_baton)) != SVN_NO_ERROR)
+    {
+      *py_pool = NULL;
+      goto finished;
+    }
+  if ((*py_pool = PyObject_GetAttrString(item_baton, "pool")) == NULL)
+    {
+      err = callback_exception_error();
+      *editor = NULL;
+      *baton = NULL;
+      goto finished;
+    }
+  err = SVN_NO_ERROR;
+ finished:
+  Py_XDECREF(*py_pool);
+  return err;
+}
+
+static svn_error_t *
+close_baton(void *baton, const char *method, svn_boolean_t without_item)
+{
+  PyObject *editor = NULL, *baton_item = NULL;
+  PyObject *ib = baton;
   PyObject *result;
   svn_error_t *err;
 
   svn_swig_py_acquire_py_lock();
 
+  if ((err = unwrap_item_baton(&editor, &baton_item, ib)) != SVN_NO_ERROR)
+    {
+      goto finished;
+    }
+  if (without_item)
+    {
+      baton_item = NULL;
+    }
   /* If there is no baton object, then it is an edit_baton, and we should
      not bother to pass an object. Note that we still shove a NULL onto
      the stack, but the format specified just won't reference it.  */
   /* ### python doesn't have 'const' on the method name and format */
-  if ((result = PyObject_CallMethod(ib->editor, (char *)method,
-                                    ib->baton ? (char *)"(O)" : NULL,
-                                    ib->baton)) == NULL)
+  if ((result = PyObject_CallMethod(editor, (char *)method,
+                                    baton_item ? (char *)"(O)" : NULL,
+                                    baton_item)) == NULL)
     {
       err = callback_exception_error();
       goto finished;
@@ -1808,19 +1861,24 @@ static svn_error_t *close_baton(void *ba
   /* there is no return value, so just toss this object (probably Py_None) */
   Py_DECREF(result);
 
-  /* Release the editor object */
-  Py_DECREF(ib->editor);
-
-  /* We're now done with the baton. Since there isn't really a free, all
-     we need to do is note that its objects are no longer referenced by
-     the baton.  */
-  Py_XDECREF(ib->baton);
-
-#ifdef SVN_DEBUG
-  ib->editor = ib->baton = NULL;
-#endif
-
-  err = SVN_NO_ERROR;
+  /* We're now done with the baton. Release it from ancestor's dict */
+  if (PyObject_HasAttrString(ib, "release_self"))
+    {
+      /* Get reference for ib, because following function call remove
+         ib object from ancestor's dict, which we borrow the reference */
+      Py_INCREF(ib);
+      result = PyObject_CallMethod(ib, "release_self", NULL, NULL);
+      /* Now we can release the reference safely */
+      Py_DECREF(ib);
+      if (result == NULL)
+        {
+          err = callback_exception_error();
+          goto finished;
+        }
+        /* there is no return value, so just toss this object
+           (probably Py_None) */
+        Py_DECREF(result);
+    }
 
  finished:
   svn_swig_py_release_py_lock();
@@ -1831,14 +1889,19 @@ static svn_error_t *set_target_revision(
                                         svn_revnum_t target_revision,
                                         apr_pool_t *pool)
 {
-  item_baton *ib = edit_baton;
+  PyObject *editor = NULL, *baton_item = NULL;
+  PyObject *ib = edit_baton;
   PyObject *result;
   svn_error_t *err;
 
   svn_swig_py_acquire_py_lock();
 
+  if ((err = unwrap_item_baton(&editor, &baton_item, ib)) != SVN_NO_ERROR)
+    {
+      goto finished;
+    }
   /* ### python doesn't have 'const' on the method name and format */
-  if ((result = PyObject_CallMethod(ib->editor, (char *)"set_target_revision",
+  if ((result = PyObject_CallMethod(editor, (char *)"set_target_revision",
                                     (char *)"l", target_revision)) == NULL)
     {
       err = callback_exception_error();
@@ -1859,14 +1922,19 @@ static svn_error_t *open_root(void *edit
                               apr_pool_t *dir_pool,
                               void **root_baton)
 {
-  item_baton *ib = edit_baton;
-  PyObject *result;
+  PyObject *editor = NULL, *baton_item = NULL;
+  PyObject *ib = edit_baton;
+  PyObject *result = NULL;
   svn_error_t *err;
 
   svn_swig_py_acquire_py_lock();
 
+  if ((err = unwrap_item_baton(&editor, &baton_item, ib)) != SVN_NO_ERROR)
+    {
+      goto finished;
+    }
   /* ### python doesn't have 'const' on the method name and format */
-  if ((result = PyObject_CallMethod(ib->editor, (char *)"open_root",
+  if ((result = PyObject_CallMethod(editor, (char *)"open_root",
                                     (char *)"lO&", base_revision,
                                     make_ob_pool, dir_pool)) == NULL)
     {
@@ -1874,11 +1942,15 @@ static svn_error_t *open_root(void *edit
       goto finished;
     }
 
-  /* make_baton takes our 'result' reference */
-  *root_baton = make_baton(dir_pool, ib->editor, result);
+  if ((*root_baton = make_baton(dir_pool, ib, result)) == NULL)
+    {
+      err = callback_exception_error();
+      goto finished;
+    }
   err = SVN_NO_ERROR;
 
  finished:
+  Py_XDECREF(result);
   svn_swig_py_release_py_lock();
   return err;
 }
@@ -1888,16 +1960,21 @@ static svn_error_t *delete_entry(const c
                                  void *parent_baton,
                                  apr_pool_t *pool)
 {
-  item_baton *ib = parent_baton;
+  PyObject *editor = NULL, *baton_item = NULL;
+  PyObject *ib = parent_baton;
   PyObject *result;
   svn_error_t *err;
 
   svn_swig_py_acquire_py_lock();
 
+  if ((err = unwrap_item_baton(&editor, &baton_item, ib)) != SVN_NO_ERROR)
+    {
+      goto finished;
+    }
   /* ### python doesn't have 'const' on the method name and format */
-  if ((result = PyObject_CallMethod(ib->editor, (char *)"delete_entry",
+  if ((result = PyObject_CallMethod(editor, (char *)"delete_entry",
                                     (char *)SVN_SWIG_BYTES_FMT "lOO&",
-                                    path, revision, ib->baton,
+                                    path, revision, baton_item,
                                     make_ob_pool, pool)) == NULL)
     {
       err = callback_exception_error();
@@ -1920,20 +1997,25 @@ static svn_error_t *add_directory(const
                                   apr_pool_t *dir_pool,
                                   void **child_baton)
 {
-  item_baton *ib = parent_baton;
-  PyObject *result;
+  PyObject *editor = NULL, *baton_item = NULL;
+  PyObject *ib = parent_baton;
+  PyObject *result = NULL;
   svn_error_t *err;
 
   svn_swig_py_acquire_py_lock();
 
+  if ((err = unwrap_item_baton(&editor, &baton_item, ib)) != SVN_NO_ERROR)
+    {
+      goto finished;
+    }
   /* ### python doesn't have 'const' on the method name and format */
-  if ((result = PyObject_CallMethod(ib->editor, (char *)"add_directory",
+  if ((result = PyObject_CallMethod(editor, (char *)"add_directory",
 #if IS_PY3
                                     (char *)"yOylO&",
 #else
                                     (char *)"sOslO&",
 #endif
-                                    path, ib->baton,
+                                    path, baton_item,
                                     copyfrom_path, copyfrom_revision,
                                     make_ob_pool, dir_pool)) == NULL)
     {
@@ -1941,11 +2023,15 @@ static svn_error_t *add_directory(const
       goto finished;
     }
 
-  /* make_baton takes our 'result' reference */
-  *child_baton = make_baton(dir_pool, ib->editor, result);
+  if ((*child_baton = make_baton(dir_pool, ib, result)) == NULL)
+    {
+      err = callback_exception_error();
+      goto finished;
+    }
   err = SVN_NO_ERROR;
 
  finished:
+  Py_XDECREF(result);
   svn_swig_py_release_py_lock();
   return err;
 }
@@ -1956,27 +2042,36 @@ static svn_error_t *open_directory(const
                                    apr_pool_t *dir_pool,
                                    void **child_baton)
 {
-  item_baton *ib = parent_baton;
-  PyObject *result;
+  PyObject *editor = NULL, *baton_item = NULL;
+  PyObject *ib = parent_baton;
+  PyObject *result = NULL;
   svn_error_t *err;
 
   svn_swig_py_acquire_py_lock();
 
+  if ((err = unwrap_item_baton(&editor, &baton_item, ib)) != SVN_NO_ERROR)
+    {
+      goto finished;
+    }
   /* ### python doesn't have 'const' on the method name and format */
-  if ((result = PyObject_CallMethod(ib->editor, (char *)"open_directory",
+  if ((result = PyObject_CallMethod(editor, (char *)"open_directory",
                                     (char *)SVN_SWIG_BYTES_FMT "OlO&",
-                                    path, ib->baton, base_revision,
+                                    path, baton_item, base_revision,
                                     make_ob_pool, dir_pool)) == NULL)
     {
       err = callback_exception_error();
       goto finished;
     }
 
-  /* make_baton takes our 'result' reference */
-  *child_baton = make_baton(dir_pool, ib->editor, result);
+  if ((*child_baton = make_baton(dir_pool, ib, result)) == NULL)
+    {
+      err = callback_exception_error();
+      goto finished;
+    }
   err = SVN_NO_ERROR;
 
  finished:
+  Py_XDECREF(result);
   svn_swig_py_release_py_lock();
   return err;
 }
@@ -1986,20 +2081,25 @@ static svn_error_t *change_dir_prop(void
                                     const svn_string_t *value,
                                     apr_pool_t *pool)
 {
-  item_baton *ib = dir_baton;
+  PyObject *editor = NULL, *baton_item = NULL;
+  PyObject *ib = dir_baton;
   PyObject *result;
   svn_error_t *err;
 
   svn_swig_py_acquire_py_lock();
 
+  if ((err = unwrap_item_baton(&editor, &baton_item, ib)) != SVN_NO_ERROR)
+    {
+      goto finished;
+    }
   /* ### python doesn't have 'const' on the method name and format */
-  if ((result = PyObject_CallMethod(ib->editor, (char *)"change_dir_prop",
+  if ((result = PyObject_CallMethod(editor, (char *)"change_dir_prop",
 #if IS_PY3
                                     (char *)"Oyy#O&",
 #else
                                     (char *)"Oss#O&",
 #endif
-                                    ib->baton, name,
+                                    baton_item, name,
                                     value ? value->data : NULL,
                                     (Py_ssize_t) (value ? value->len : 0),
                                     make_ob_pool, pool)) == NULL)
@@ -2020,7 +2120,7 @@ static svn_error_t *change_dir_prop(void
 static svn_error_t *close_directory(void *dir_baton,
                                     apr_pool_t *pool)
 {
-  return close_baton(dir_baton, "close_directory");
+  return close_baton(dir_baton, "close_directory", FALSE);
 }
 
 static svn_error_t *add_file(const char *path,
@@ -2030,20 +2130,25 @@ static svn_error_t *add_file(const char
                              apr_pool_t *file_pool,
                              void **file_baton)
 {
-  item_baton *ib = parent_baton;
-  PyObject *result;
+  PyObject *editor = NULL, *baton_item = NULL;
+  PyObject *ib = parent_baton;
+  PyObject *result = NULL;
   svn_error_t *err;
 
   svn_swig_py_acquire_py_lock();
 
+  if ((err = unwrap_item_baton(&editor, &baton_item, ib)) != SVN_NO_ERROR)
+    {
+      goto finished;
+    }
   /* ### python doesn't have 'const' on the method name and format */
-  if ((result = PyObject_CallMethod(ib->editor, (char *)"add_file",
+  if ((result = PyObject_CallMethod(editor, (char *)"add_file",
 #if IS_PY3
                                     (char *)"yOylO&",
 #else
                                     (char *)"sOslO&",
 #endif
-                                    path, ib->baton,
+                                    path, baton_item,
                                     copyfrom_path, copyfrom_revision,
                                     make_ob_pool, file_pool)) == NULL)
     {
@@ -2051,12 +2156,16 @@ static svn_error_t *add_file(const char
       goto finished;
     }
 
-  /* make_baton takes our 'result' reference */
-  *file_baton = make_baton(file_pool, ib->editor, result);
+  if ((*file_baton = make_baton(file_pool, ib, result)) == NULL)
+    {
+      err = callback_exception_error();
+      goto finished;
+    }
 
   err = SVN_NO_ERROR;
 
  finished:
+  Py_XDECREF(result);
   svn_swig_py_release_py_lock();
   return err;
 }
@@ -2067,27 +2176,36 @@ static svn_error_t *open_file(const char
                               apr_pool_t *file_pool,
                               void **file_baton)
 {
-  item_baton *ib = parent_baton;
-  PyObject *result;
+  PyObject *editor = NULL, *baton_item = NULL;
+  PyObject *ib = parent_baton;
+  PyObject *result = NULL;
   svn_error_t *err;
 
   svn_swig_py_acquire_py_lock();
 
+  if ((err = unwrap_item_baton(&editor, &baton_item, ib)) != SVN_NO_ERROR)
+    {
+      goto finished;
+    }
   /* ### python doesn't have 'const' on the method name and format */
-  if ((result = PyObject_CallMethod(ib->editor, (char *)"open_file",
+  if ((result = PyObject_CallMethod(editor, (char *)"open_file",
                                     (char *)SVN_SWIG_BYTES_FMT "OlO&",
-                                    path, ib->baton, base_revision,
+                                    path, baton_item, base_revision,
                                     make_ob_pool, file_pool)) == NULL)
     {
       err = callback_exception_error();
       goto finished;
     }
 
-  /* make_baton takes our 'result' reference */
-  *file_baton = make_baton(file_pool, ib->editor, result);
+  if ((*file_baton = make_baton(file_pool, ib, result)) == NULL)
+    {
+      err = callback_exception_error();
+      goto finished;
+    }
   err = SVN_NO_ERROR;
 
  finished:
+  Py_XDECREF(result);
   svn_swig_py_release_py_lock();
   return err;
 }
@@ -2095,12 +2213,19 @@ static svn_error_t *open_file(const char
 static svn_error_t *window_handler(svn_txdelta_window_t *window,
                                    void *baton)
 {
-  PyObject *handler = baton;
-  PyObject *result;
-  svn_error_t *err;
+  PyObject *editor = NULL, *handler = NULL;
+  PyObject *ib = baton;
+  PyObject *result = NULL;
+  int is_last_call = FALSE;
+  svn_error_t *err = SVN_NO_ERROR;
 
   svn_swig_py_acquire_py_lock();
 
+  if ((err = unwrap_item_baton(&editor, &handler, ib)) != SVN_NO_ERROR)
+    {
+      is_last_call = TRUE;
+      goto finished;
+    }
   if (window == NULL)
     {
       /* the last call; it closes the handler */
@@ -2108,9 +2233,8 @@ static svn_error_t *window_handler(svn_t
       /* invoke the handler with None for the window */
       /* ### python doesn't have 'const' on the format */
       result = PyObject_CallFunction(handler, (char *)"O", Py_None);
+      is_last_call = TRUE;
 
-      /* we no longer need to refer to the handler object */
-      Py_DECREF(handler);
     }
   else
     {
@@ -2123,14 +2247,40 @@ static svn_error_t *window_handler(svn_t
   if (result == NULL)
     {
       err = callback_exception_error();
+      is_last_call = TRUE;
       goto finished;
     }
-
-  /* there is no return value, so just toss this object (probably Py_None) */
-  Py_DECREF(result);
-  err = SVN_NO_ERROR;
+  else
+    {
+      /* there is no return value, so just toss this object
+         (probably Py_None) */
+      Py_DECREF(result);
+      err = SVN_NO_ERROR;
+    }
 
  finished:
+  if (is_last_call)
+    {
+      /* now we should release the handler object */
+      if (PyObject_HasAttrString(ib, "release_self"))
+        {
+          /* Get reference for ib, because following function call remove
+             ib object from ancestor's dict, which we borrow the reference */
+          Py_INCREF(ib);
+          result = PyObject_CallMethod(ib, "release_self", NULL, NULL);
+          /* Now we can release the reference safely */
+          Py_DECREF(ib);
+          if (result == NULL)
+            {
+              if (err == SVN_NO_ERROR)
+                {
+                  err = callback_exception_error();
+                }
+            }
+          Py_XDECREF(result);
+        }
+    }
+
   svn_swig_py_release_py_lock();
   return err;
 }
@@ -2141,20 +2291,25 @@ static svn_error_t *apply_textdelta(void
                                     svn_txdelta_window_handler_t *handler,
                                     void **h_baton)
 {
-  item_baton *ib = file_baton;
-  PyObject *result;
+  PyObject *editor = NULL, *baton_item = NULL;
+  PyObject *ib = file_baton;
+  PyObject *result = NULL;
   svn_error_t *err;
 
   svn_swig_py_acquire_py_lock();
 
+  if ((err = unwrap_item_baton(&editor, &baton_item, ib)) != SVN_NO_ERROR)
+    {
+      goto finished;
+    }
   /* ### python doesn't have 'const' on the method name and format */
-  if ((result = PyObject_CallMethod(ib->editor, (char *)"apply_textdelta",
+  if ((result = PyObject_CallMethod(editor, (char *)"apply_textdelta",
 #if IS_PY3
                                     (char *)"(Oy)",
 #else
                                     (char *)"(Os)",
 #endif
-                                    ib->baton,
+                                    baton_item,
                                     base_checksum)) == NULL)
     {
       err = callback_exception_error();
@@ -2173,15 +2328,21 @@ static svn_error_t *apply_textdelta(void
     }
   else
     {
-      /* return the thunk for invoking the handler. the baton takes our
-         'result' reference, which is the handler. */
+      /* 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. */
       *handler = window_handler;
-      *h_baton = result;
+      if ((*h_baton = make_baton(pool, ib, result)) == NULL)
+        {
+          err = callback_exception_error();
+          goto finished;
+        }
     }
 
   err = SVN_NO_ERROR;
 
  finished:
+  Py_XDECREF(result);
   svn_swig_py_release_py_lock();
   return err;
 }
@@ -2191,20 +2352,25 @@ static svn_error_t *change_file_prop(voi
                                      const svn_string_t *value,
                                      apr_pool_t *pool)
 {
-  item_baton *ib = file_baton;
+  PyObject *editor = NULL, *baton_item = NULL;
+  PyObject *ib = file_baton;
   PyObject *result;
   svn_error_t *err;
 
   svn_swig_py_acquire_py_lock();
 
+  if ((err = unwrap_item_baton(&editor, &baton_item, ib)) != SVN_NO_ERROR)
+    {
+      goto finished;
+    }
   /* ### python doesn't have 'const' on the method name and format */
-  if ((result = PyObject_CallMethod(ib->editor, (char *)"change_file_prop",
+  if ((result = PyObject_CallMethod(editor, (char *)"change_file_prop",
 #if IS_PY3
                                     (char *)"Oyy#O&",
 #else
                                     (char *)"Oss#O&",
 #endif
-                                    ib->baton, name,
+                                    baton_item, name,
                                     value ? value->data : NULL,
                                     (Py_ssize_t) (value ? value->len : 0),
                                     make_ob_pool, pool)) == NULL)
@@ -2226,20 +2392,25 @@ static svn_error_t *close_file(void *fil
                                const char *text_checksum,
                                apr_pool_t *pool)
 {
-  item_baton *ib = file_baton;
+  PyObject *editor = NULL, *baton_item = NULL;
+  PyObject *ib = file_baton;
   PyObject *result;
   svn_error_t *err;
 
   svn_swig_py_acquire_py_lock();
 
+  if ((err = unwrap_item_baton(&editor, &baton_item, ib)) != SVN_NO_ERROR)
+    {
+      goto finished;
+    }
   /* ### python doesn't have 'const' on the method name and format */
-  if ((result = PyObject_CallMethod(ib->editor, (char *)"close_file",
+  if ((result = PyObject_CallMethod(editor, (char *)"close_file",
 #if IS_PY3
                                     (char *)"(Oy)",
 #else
                                     (char *)"(Os)",
 #endif
-                                    ib->baton,
+                                    baton_item,
                                     text_checksum)) == NULL)
     {
       err = callback_exception_error();
@@ -2249,14 +2420,24 @@ static svn_error_t *close_file(void *fil
   /* there is no return value, so just toss this object (probably Py_None) */
   Py_DECREF(result);
 
-  /* We're now done with the baton. Since there isn't really a free, all
-     we need to do is note that its objects are no longer referenced by
-     the baton.  */
-  Py_XDECREF(ib->baton);
-
-#ifdef SVN_DEBUG
-  ib->editor = ib->baton = NULL;
-#endif
+  /* We're now done with the baton. Release it from ancestor's dict */
+  if (PyObject_HasAttrString(ib, "release_self"))
+    {
+      /* Get reference for ib, because following function call remove
+         ib object from ancestor's dict, which we borrow the reference */
+      Py_INCREF(ib);
+      result = PyObject_CallMethod(ib, "release_self", NULL, NULL);
+      /* Now we can release the reference safely */
+      Py_DECREF(ib);
+      if (result == NULL)
+        {
+          err = callback_exception_error();
+          goto finished;
+        }
+        /* there is no return value, so just toss this object
+           (probably Py_None) */
+        Py_DECREF(result);
+    }
 
   err = SVN_NO_ERROR;
 
@@ -2268,18 +2449,16 @@ static svn_error_t *close_file(void *fil
 static svn_error_t *close_edit(void *edit_baton,
                                apr_pool_t *pool)
 {
-  return close_baton(edit_baton, "close_edit");
+  return close_baton(edit_baton, "close_edit", TRUE);
 }
 
 static svn_error_t *abort_edit(void *edit_baton,
                                apr_pool_t *pool)
 {
-  return close_baton(edit_baton, "abort_edit");
+  return close_baton(edit_baton, "abort_edit", TRUE);
 }
 
 void svn_swig_py_make_editor(const svn_delta_editor_t **editor,
-                             void **edit_baton,
-                             PyObject *py_editor,
                              apr_pool_t *pool)
 {
   svn_delta_editor_t *thunk_editor = svn_delta_default_editor(pool);
@@ -2300,7 +2479,6 @@ void svn_swig_py_make_editor(const svn_d
   thunk_editor->abort_edit = abort_edit;
 
   *editor = thunk_editor;
-  *edit_baton = make_baton(pool, py_editor, NULL);
 }
 
 
@@ -2310,14 +2488,19 @@ static svn_error_t *parse_fn3_magic_head
                                                   void *parse_baton,
                                                   apr_pool_t *pool)
 {
-  item_baton *ib = parse_baton;
+  PyObject *editor = NULL, *baton_item = NULL;
+  PyObject *ib = parse_baton;
   PyObject *result;
   svn_error_t *err;
 
   svn_swig_py_acquire_py_lock();
 
+  if ((err = unwrap_item_baton(&editor, &baton_item, ib)) != SVN_NO_ERROR)
+    {
+      goto finished;
+    }
   /* ### python doesn't have 'const' on the method name and format */
-  if ((result = PyObject_CallMethod(ib->editor, (char *)"magic_header_record",
+  if ((result = PyObject_CallMethod(editor, (char *)"magic_header_record",
                                     (char *)"lO&", version,
                                     make_ob_pool, pool)) == NULL)
     {
@@ -2339,14 +2522,19 @@ static svn_error_t *parse_fn3_uuid_recor
                                           void *parse_baton,
                                           apr_pool_t *pool)
 {
-  item_baton *ib = parse_baton;
+  PyObject *editor = NULL, *baton_item = NULL;
+  PyObject *ib = parse_baton;
   PyObject *result;
   svn_error_t *err;
 
   svn_swig_py_acquire_py_lock();
 
+  if ((err = unwrap_item_baton(&editor, &baton_item, ib)) != SVN_NO_ERROR)
+    {
+      goto finished;
+    }
   /* ### python doesn't have 'const' on the method name and format */
-  if ((result = PyObject_CallMethod(ib->editor, (char *)"uuid_record",
+  if ((result = PyObject_CallMethod(editor, (char *)"uuid_record",
                                     (char *)SVN_SWIG_BYTES_FMT "O&", uuid,
                                     make_ob_pool, pool)) == NULL)
     {
@@ -2369,14 +2557,19 @@ static svn_error_t *parse_fn3_new_revisi
                                                   void *parse_baton,
                                                   apr_pool_t *pool)
 {
-  item_baton *ib = parse_baton;
-  PyObject *result;
+  PyObject *editor = NULL, *baton_item = NULL;
+  PyObject *ib = parse_baton;
+  PyObject *result = NULL;
   PyObject *tmp;
   svn_error_t *err;
 
   svn_swig_py_acquire_py_lock();
 
-  if ((result = PyObject_CallMethod(ib->editor, (char *)"new_revision_record",
+  if ((err = unwrap_item_baton(&editor, &baton_item, ib)) != SVN_NO_ERROR)
+    {
+      goto finished;
+    }
+  if ((result = PyObject_CallMethod(editor, (char *)"new_revision_record",
                                    (char *)"O&O&",
                                    svn_swig_py_stringhash_to_dict, headers,
                                    make_ob_pool, pool)) == NULL) {
@@ -2384,11 +2577,15 @@ static svn_error_t *parse_fn3_new_revisi
       goto finished;
     }
 
-  /* make_baton takes our 'result' reference */
-  *revision_baton = make_baton(pool, ib->editor, result);
+  if ((*revision_baton = make_baton(pool, ib, result)) == NULL)
+    {
+      err = callback_exception_error();
+      goto finished;
+    }
   err = SVN_NO_ERROR;
 
  finished:
+  Py_XDECREF(result);
   svn_swig_py_release_py_lock();
   return err;
 }
@@ -2399,26 +2596,35 @@ static svn_error_t *parse_fn3_new_node_r
                                               void *revision_baton,
                                               apr_pool_t *pool)
 {
-  item_baton *ib = revision_baton;
-  PyObject *result;
+  PyObject *editor = NULL, *baton_item = NULL;
+  PyObject *ib = revision_baton;
+  PyObject *result = NULL;
   svn_error_t *err;
 
   svn_swig_py_acquire_py_lock();
 
-  if ((result = PyObject_CallMethod(ib->editor, (char *)"new_node_record",
+  if ((err = unwrap_item_baton(&editor, &baton_item, ib)) != SVN_NO_ERROR)
+    {
+      goto finished;
+    }
+  if ((result = PyObject_CallMethod(editor, (char *)"new_node_record",
                                    (char *)"O&OO&",
                                    svn_swig_py_stringhash_to_dict, headers,
-                                   ib->baton,
+                                   baton_item,
                                    make_ob_pool, pool)) == NULL) {
       err = callback_exception_error();
       goto finished;
     }
 
-  /* make_baton takes our 'result' reference */
-  *node_baton = make_baton(pool, ib->editor, result);
+  if ((*node_baton = make_baton(pool, ib, result)) == NULL)
+    {
+      err = callback_exception_error();
+      goto finished;
+    }
   err = SVN_NO_ERROR;
 
  finished:
+  Py_XDECREF(result);
   svn_swig_py_release_py_lock();
   return err;
 }
@@ -2428,20 +2634,25 @@ static svn_error_t *parse_fn3_set_revisi
                                                     const char *name,
                                                     const svn_string_t *value)
 {
-  item_baton *ib = revision_baton;
+  PyObject *editor = NULL, *baton_item = NULL;
+  PyObject *ib = revision_baton;
   PyObject *result;
   svn_error_t *err;
 
   svn_swig_py_acquire_py_lock();
 
+  if ((err = unwrap_item_baton(&editor, &baton_item, ib)) != SVN_NO_ERROR)
+    {
+      goto finished;
+    }
   /* ### python doesn't have 'const' on the method name and format */
-  if ((result = PyObject_CallMethod(ib->editor, (char 
*)"set_revision_property",
+  if ((result = PyObject_CallMethod(editor, (char *)"set_revision_property",
 #if IS_PY3
                                     (char *)"Oyy#",
 #else
                                     (char *)"Oss#",
 #endif
-                                    ib->baton, name,
+                                    baton_item, name,
                                     value ? value->data : NULL,
                                     (Py_ssize_t) (value ? value->len : 0)))
       == NULL)
@@ -2464,20 +2675,25 @@ static svn_error_t *parse_fn3_set_node_p
                                                 const char *name,
                                                 const svn_string_t *value)
 {
-  item_baton *ib = node_baton;
+  PyObject *editor = NULL, *baton_item = NULL;
+  PyObject *ib = node_baton;
   PyObject *result;
   svn_error_t *err;
 
   svn_swig_py_acquire_py_lock();
 
+  if ((err = unwrap_item_baton(&editor, &baton_item, ib)) != SVN_NO_ERROR)
+    {
+      goto finished;
+    }
   /* ### python doesn't have 'const' on the method name and format */
-  if ((result = PyObject_CallMethod(ib->editor, (char *)"set_node_property",
+  if ((result = PyObject_CallMethod(editor, (char *)"set_node_property",
 #if IS_PY3
                                     (char *)"Oyy#",
 #else
                                     (char *)"Oss#",
 #endif
-                                    ib->baton, name,
+                                    baton_item, name,
                                     value ? value->data : NULL,
                                     (Py_ssize_t) (value ? value->len : 0)))
       == NULL)
@@ -2499,16 +2715,21 @@ static svn_error_t *parse_fn3_set_node_p
 static svn_error_t *parse_fn3_delete_node_property(void *node_baton,
                                                    const char *name)
 {
-  item_baton *ib = node_baton;
+  PyObject *editor = NULL, *baton_item = NULL;
+  PyObject *ib = node_baton;
   PyObject *result;
   svn_error_t *err;
 
   svn_swig_py_acquire_py_lock();
 
+  if ((err = unwrap_item_baton(&editor, &baton_item, ib)) != SVN_NO_ERROR)
+    {
+      goto finished;
+    }
   /* ### python doesn't have 'const' on the method name and format */
-  if ((result = PyObject_CallMethod(ib->editor, (char *)"delete_node_property",
+  if ((result = PyObject_CallMethod(editor, (char *)"delete_node_property",
                                     (char *)"O" SVN_SWIG_BYTES_FMT,
-                                    ib->baton, name)) == NULL)
+                                    baton_item, name)) == NULL)
     {
       err = callback_exception_error();
       goto finished;
@@ -2526,15 +2747,20 @@ static svn_error_t *parse_fn3_delete_nod
 
 static svn_error_t *parse_fn3_remove_node_props(void *node_baton)
 {
-  item_baton *ib = node_baton;
+  PyObject *editor = NULL, *baton_item = NULL;
+  PyObject *ib = node_baton;
   PyObject *result;
   svn_error_t *err;
 
   svn_swig_py_acquire_py_lock();
 
+  if ((err = unwrap_item_baton(&editor, &baton_item, ib)) != SVN_NO_ERROR)
+    {
+      goto finished;
+    }
   /* ### python doesn't have 'const' on the method name and format */
-  if ((result = PyObject_CallMethod(ib->editor, (char *)"remove_node_props",
-                                    (char *)"(O)", ib->baton)) == NULL)
+  if ((result = PyObject_CallMethod(editor, (char *)"remove_node_props",
+                                    (char *)"(O)", baton_item)) == NULL)
     {
       err = callback_exception_error();
       goto finished;
@@ -2553,15 +2779,22 @@ static svn_error_t *parse_fn3_remove_nod
 static svn_error_t *parse_fn3_set_fulltext(svn_stream_t **stream,
                                            void *node_baton)
 {
-  item_baton *ib = node_baton;
+  PyObject *editor = NULL, *baton_item = NULL, *py_pool = NULL;
+  PyObject *ib = node_baton;
   PyObject *result = NULL;
+  apr_pool_t *pool;
   svn_error_t *err = SVN_NO_ERROR;
 
   svn_swig_py_acquire_py_lock();
 
+  if ((err = unwrap_item_baton_with_pool(&editor, &baton_item, &py_pool,
+                                         ib)) != SVN_NO_ERROR)
+    {
+      goto finished;
+    }
   /* ### python doesn't have 'const' on the method name and format */
-  if ((result = PyObject_CallMethod(ib->editor, (char *)"set_fulltext",
-                                    (char *)"(O)", ib->baton)) == NULL)
+  if ((result = PyObject_CallMethod(editor, (char *)"set_fulltext",
+                                    (char *)"(O)", baton_item)) == NULL)
     {
       err = callback_exception_error();
       goto finished;
@@ -2574,9 +2807,15 @@ static svn_error_t *parse_fn3_set_fullte
     }
   else
     {
+      if (svn_swig_ConvertPtrString(py_pool, (void **)&pool,
+                                    "apr_pool_t *") == -1)
+        {
+          err = type_conversion_error("apr_pool_t *");
+          goto finished;
+        }
       /* create a stream from the IO object. it will increment the
          reference on the 'result'. */
-      *stream = svn_swig_py_make_stream(result, ib->pool);
+      *stream = svn_swig_py_make_stream(result, pool);
       if (*stream == NULL)
         {
           err = callback_exception_error();
@@ -2593,19 +2832,27 @@ finished:
 }
 
 
-static svn_error_t *parse_fn3_apply_textdelta(svn_txdelta_window_handler_t 
*handler,
-                                              void **handler_baton,
-                                              void *node_baton)
+static svn_error_t *
+parse_fn3_apply_textdelta(svn_txdelta_window_handler_t *handler,
+                          void **handler_baton,
+                          void *node_baton)
 {
-  item_baton *ib = node_baton;
+  PyObject *editor = NULL, *baton_item = NULL, *py_pool = NULL;
+  PyObject *ib = node_baton;
+  apr_pool_t *pool;
   PyObject *result;
   svn_error_t *err;
 
   svn_swig_py_acquire_py_lock();
 
+  if ((err = unwrap_item_baton_with_pool(&editor, &baton_item, &py_pool,
+                                         ib)) != SVN_NO_ERROR)
+    {
+      goto finished;
+    }
   /* ### python doesn't have 'const' on the method name and format */
-  if ((result = PyObject_CallMethod(ib->editor, (char *)"apply_textdelta",
-                                    (char *)"(O)", ib->baton)) == NULL)
+  if ((result = PyObject_CallMethod(editor, (char *)"apply_textdelta",
+                                    (char *)"(O)", baton_item)) == NULL)
     {
       err = callback_exception_error();
       goto finished;
@@ -2623,10 +2870,21 @@ static svn_error_t *parse_fn3_apply_text
     }
   else
     {
-      /* return the thunk for invoking the handler. the baton takes our
-         'result' reference, which is the handler. */
+      /* 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. */
       *handler = window_handler;
-      *handler_baton = result;
+      if (svn_swig_ConvertPtrString(py_pool, (void **)&pool,
+                                    "apr_pool_t *") == -1)
+        {
+          err = type_conversion_error("apr_pool_t *");
+          goto finished;
+        }
+      if ((*handler_baton = make_baton(pool, ib, result)) == NULL)
+        {
+          err = callback_exception_error();
+          goto finished;
+        }
     }
 
   err = SVN_NO_ERROR;
@@ -2639,13 +2897,13 @@ static svn_error_t *parse_fn3_apply_text
 
 static svn_error_t *parse_fn3_close_node(void *node_baton)
 {
-  return close_baton(node_baton, "close_node");
+  return close_baton(node_baton, "close_node", FALSE);
 }
 
 
 static svn_error_t *parse_fn3_close_revision(void *revision_baton)
 {
-  return close_baton(revision_baton, "close_revision");
+  return close_baton(revision_baton, "close_revision", FALSE);
 }
 
 
@@ -2665,26 +2923,11 @@ static const svn_repos_parse_fns3_t thun
     parse_fn3_close_revision
   };
 
-static apr_status_t
-svn_swig_py_parse_fns3_destroy(void *parse_baton)
-{
-  close_baton(parse_baton, "_close_dumpstream");
-  return APR_SUCCESS;
-}
-
 void svn_swig_py_make_parse_fns3(const svn_repos_parse_fns3_t **parse_fns3,
-                                 void **parse_baton,
-                                 PyObject *py_parse_fns3,
                                  apr_pool_t *pool)
 {
   *parse_fns3 = &thunk_parse_fns3_vtable;
-  *parse_baton = make_baton(pool, py_parse_fns3, NULL);
-
-  /* Dump stream vtable does not provide a method which is called right before
-     the end of the parsing (similar to close_edit/abort_edit in delta editor).
-     Thus, register a pool clean-up routine to release this parse baton. */
-  apr_pool_cleanup_register(pool, *parse_baton, svn_swig_py_parse_fns3_destroy,
-                            apr_pool_cleanup_null);
+  return;
 }
 
 

Modified: 
subversion/branches/1.14.x/subversion/bindings/swig/python/libsvn_swig_py/swigutil_py.h
URL: 
http://svn.apache.org/viewvc/subversion/branches/1.14.x/subversion/bindings/swig/python/libsvn_swig_py/swigutil_py.h?rev=1913135&r1=1913134&r2=1913135&view=diff
==============================================================================
--- 
subversion/branches/1.14.x/subversion/bindings/swig/python/libsvn_swig_py/swigutil_py.h
 (original)
+++ 
subversion/branches/1.14.x/subversion/bindings/swig/python/libsvn_swig_py/swigutil_py.h
 Fri Oct 20 04:00:04 2023
@@ -268,14 +268,10 @@ svn_swig_py_unwrap_struct_ptr(PyObject *
 
 /* make an editor that "thunks" from C callbacks up to Python */
 void svn_swig_py_make_editor(const svn_delta_editor_t **editor,
-                             void **edit_baton,
-                             PyObject *py_editor,
                              apr_pool_t *pool);
 
 /* make a parse vtable that "thunks" from C callbacks up to Python */
 void svn_swig_py_make_parse_fns3(const svn_repos_parse_fns3_t **parse_fns3,
-                                 void **parse_baton,
-                                 PyObject *py_parse_fns3,
                                  apr_pool_t *pool);
 
 apr_file_t *svn_swig_py_make_file(PyObject *py_file,

Modified: 
subversion/branches/1.14.x/subversion/bindings/swig/python/svn/delta.py
URL: 
http://svn.apache.org/viewvc/subversion/branches/1.14.x/subversion/bindings/swig/python/svn/delta.py?rev=1913135&r1=1913134&r2=1913135&view=diff
==============================================================================
--- subversion/branches/1.14.x/subversion/bindings/swig/python/svn/delta.py 
(original)
+++ subversion/branches/1.14.x/subversion/bindings/swig/python/svn/delta.py Fri 
Oct 20 04:00:04 2023
@@ -77,5 +77,6 @@ class Editor:
     pass
 
 
-def make_editor(editor, pool=None):
-  return svn_swig_py_make_editor(editor, pool)
+def make_editor(editor, pool=None, baton=None):
+  from libsvn.delta import _AncBaton
+  return svn_swig_py_make_editor(pool), _AncBaton(editor, pool, baton)

Modified: 
subversion/branches/1.14.x/subversion/bindings/swig/python/svn/repos.py
URL: 
http://svn.apache.org/viewvc/subversion/branches/1.14.x/subversion/bindings/swig/python/svn/repos.py?rev=1913135&r1=1913134&r2=1913135&view=diff
==============================================================================
--- subversion/branches/1.14.x/subversion/bindings/swig/python/svn/repos.py 
(original)
+++ subversion/branches/1.14.x/subversion/bindings/swig/python/svn/repos.py Fri 
Oct 20 04:00:04 2023
@@ -336,5 +336,16 @@ class ParseFns3:
         pass
 
 
-def make_parse_fns3(parse_fns3, pool=None):
-    return svn_swig_py_make_parse_fns3(parse_fns3, pool)
+def make_parse_fns3(parse_fns3, pool=None, baton=None):
+  from libsvn.delta import _AncBaton
+
+  class _ParseBaton(_AncBaton):
+    # Drive _close_dumpstream method when the instance is deleted.
+    # For backward compatibility before Subversion 1.15, we call it even if
+    # the instance would not be used by C API, or the C API would cause
+    # some error.
+    def __del__(self):
+      self.editor._close_dumpstream()
+
+  parse_baton = _ParseBaton(parse_fns3, pool, baton)
+  return svn_swig_py_make_parse_fns3(pool), parse_baton

Modified: 
subversion/branches/1.14.x/subversion/bindings/swig/python/tests/delta.py
URL: 
http://svn.apache.org/viewvc/subversion/branches/1.14.x/subversion/bindings/swig/python/tests/delta.py?rev=1913135&r1=1913134&r2=1913135&view=diff
==============================================================================
--- subversion/branches/1.14.x/subversion/bindings/swig/python/tests/delta.py 
(original)
+++ subversion/branches/1.14.x/subversion/bindings/swig/python/tests/delta.py 
Fri Oct 20 04:00:04 2023
@@ -21,6 +21,7 @@
 import unittest, setup_path
 import os
 import tempfile
+import weakref
 import svn.delta
 import svn.core
 from sys import version_info # For Python version check
@@ -117,6 +118,19 @@ class DeltaTestCase(unittest.TestCase):
     # Check that the ops inherit the window's pool
     self.assertEqual(window.ops[0]._parent_pool, window._parent_pool)
 
+  def testMakeEditorLeak(self):
+    """Issue 4916, check ref-count leak on svn.delta.make_editor()"""
+    pool = svn.core.Pool()
+    editor = svn.delta.Editor()
+    editor_ref = weakref.ref(editor)
+    e_ptr, e_baton = svn.delta.make_editor(editor, pool)
+    del e_ptr, e_baton
+    self.assertNotEqual(editor_ref(), None)
+    del pool
+    self.assertNotEqual(editor_ref(), None)
+    del editor
+    self.assertEqual(editor_ref(), None)
+
 def suite():
   return unittest.defaultTestLoader.loadTestsFromTestCase(DeltaTestCase)
 

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=1913135&r1=1913134&r2=1913135&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 
Fri Oct 20 04:00:04 2023
@@ -18,11 +18,11 @@
 # under the License.
 #
 #
-import unittest, setup_path, os, sys
+import unittest, setup_path, os, sys, weakref
 from sys import version_info # For Python version check
 from io import BytesIO
 from svn import core, repos, fs, delta
-from svn.core import SubversionException
+from svn.core import SubversionException, Pool
 import utils
 
 class ChangeReceiver(delta.Editor):
@@ -40,9 +40,20 @@ class ChangeReceiver(delta.Editor):
     return textdelta_handler
 
 class DumpStreamParser(repos.ParseFns3):
-  def __init__(self):
+  def __init__(self, stream=None, pool=None):
     repos.ParseFns3.__init__(self)
+    self.stream = stream
     self.ops = []
+    # for leak checking only. If the parse_fns3 object holds some proxy
+    # object allocated from 'pool' or the 'pool' itself, the 'pool' is not
+    # destroyed until the parse_fns3 object is removed.
+    self.pool = pool
+  def _close_dumpstream(self):
+    if self.stream:
+      self.stream.close()
+      self.stream = None
+    if self.pool:
+      self.pool = None
   def magic_header_record(self, version, pool=None):
     self.ops.append((b"magic-header", version))
   def uuid_record(self, uuid, pool=None):
@@ -74,6 +85,76 @@ class DumpStreamParser(repos.ParseFns3):
     self.ops.append((b"set-fulltext", node_baton[0], node_baton[1]))
     return None
 
+class BatonCollector(repos.ChangeCollector):
+  """A ChangeCollector with collecting batons, too"""
+  def __init__(self, fs_ptr, root, pool=None, notify_cb=None):
+    repos.ChangeCollector.__init__(self, fs_ptr, root, pool, notify_cb)
+    self.batons = []
+    self.close_called = False
+    self.abort_called = False
+
+  def open_root(self, base_revision, dir_pool=None):
+    bt = repos.ChangeCollector.open_root(self, base_revision, dir_pool)
+    self.batons.append((b'dir baton', b'', bt, sys.getrefcount(bt)))
+    return bt
+
+  def add_directory(self, path, parent_baton,
+                    copyfrom_path, copyfrom_revision, dir_pool=None):
+    bt = repos.ChangeCollector.add_directory(self, path, parent_baton,
+                                             copyfrom_path,
+                                             copyfrom_revision,
+                                             dir_pool)
+    self.batons.append((b'dir baton', path, bt, sys.getrefcount(bt)))
+    return bt
+
+  def open_directory(self, path, parent_baton, base_revision,
+                     dir_pool=None):
+    bt = repos.ChangeCollector.open_directory(self, path, parent_baton,
+                                              base_revision, dir_pool)
+    self.batons.append((b'dir baton', path, bt, sys.getrefcount(bt)))
+    return bt
+
+  def add_file(self, path, parent_baton,
+               copyfrom_path, copyfrom_revision, file_pool=None):
+    bt = repos.ChangeCollector.add_file(self, path, parent_baton,
+                                        copyfrom_path, copyfrom_revision,
+                                        file_pool)
+    self.batons.append((b'file baton', path, bt, sys.getrefcount(bt)))
+    return bt
+
+  def open_file(self, path, parent_baton, base_revision, file_pool=None):
+    bt = repos.ChangeCollector.open_file(self, path, parent_baton,
+                                         base_revision, file_pool)
+    self.batons.append((b'file baton', path, bt, sys.getrefcount(bt)))
+    return bt
+
+  def close_edit(self, pool=None):
+    self.close_called = True
+    return
+
+  def abort_edit(self, pool=None):
+    self.abort_called = True
+    return
+
+class BatonCollectorErrorOnClose(BatonCollector):
+  """Same as BatonCollector, but raises an Exception when close the
+     file/dir specfied by error_path"""
+  def __init__(self, fs_ptr, root, pool=None, notify_cb=None, error_path=b''):
+    BatonCollector.__init__(self, fs_ptr, root, pool, notify_cb)
+    self.error_path = error_path
+
+  def close_directory(self, dir_baton):
+    if dir_baton[0] == self.error_path:
+      raise SubversionException('A Dummy Exception!', core.SVN_ERR_BASE)
+    else:
+      BatonCollector.close_directory(self, dir_baton)
+
+  def close_file(self, file_baton, text_checksum):
+    if file_baton[0] == self.error_path:
+      raise SubversionException('A Dummy Exception!', core.SVN_ERR_BASE)
+    else:
+      return BatonCollector.close_file(self, file_baton, text_checksum)
+
 
 def _authz_callback(root, path, pool):
   "A dummy authz callback which always returns success."
@@ -175,13 +256,15 @@ class SubversionRepositoryTestCase(unitt
     def is_cancelled():
       self.cancel_calls += 1
       return None
+    pool = Pool()
+    subpool = Pool(pool)
     dump_path = os.path.join(os.path.dirname(sys.argv[0]),
         "trac/versioncontrol/tests/svnrepos.dump")
     stream = open(dump_path, 'rb')
-    dsp = DumpStreamParser()
-    ptr, baton = repos.make_parse_fns3(dsp)
+    dsp = DumpStreamParser(stream, subpool)
+    dsp_ref = weakref.ref(dsp)
+    ptr, baton = repos.make_parse_fns3(dsp, subpool)
     repos.parse_dumpstream3(stream, ptr, baton, False, is_cancelled)
-    stream.close()
     self.assertEqual(self.cancel_calls, 76)
     expected_list = [
         (b"magic-header", 2),
@@ -226,6 +309,13 @@ class SubversionRepositoryTestCase(unitt
     # the comparison list gets too long.
     self.assertEqual(dsp.ops[:len(expected_list)], expected_list)
 
+    # _close_dumpstream should be invoked after 'baton' is removed.
+    self.assertEqual(False, stream.closed)
+    del ptr, baton, subpool, dsp
+    self.assertEqual(True, stream.closed)
+    # Issue SVN-4918
+    self.assertEqual(None, dsp_ref())
+
   def test_parse_fns3_invalid_set_fulltext(self):
     class DumpStreamParserSubclass(DumpStreamParser):
       def set_fulltext(self, node_baton):
@@ -290,6 +380,53 @@ class SubversionRepositoryTestCase(unitt
       repos.dir_delta(prev_root, b'', b'', this_root, b'', e_ptr, e_baton,
               _authz_callback, 1, 1, 0, 0)
 
+  def test_delta_editor_leak_with_change_collector(self):
+    pool = Pool()
+    subpool = Pool(pool)
+    root = fs.revision_root(self.fs, self.rev, subpool)
+    editor = repos.ChangeCollector(self.fs, root, subpool)
+    editor_ref = weakref.ref(editor)
+    e_ptr, e_baton = delta.make_editor(editor, subpool)
+    repos.replay(root, e_ptr, e_baton, subpool)
+
+    fs.close_root(root)
+    del root
+    self.assertNotEqual(None, editor_ref())
+
+    del e_ptr, e_baton, editor
+    del subpool
+    self.assertEqual(None, editor_ref())
+
+  def test_replay_batons_refcounts(self):
+    """Issue SVN-4917: check ref-count of batons created and used in 
callbacks"""
+    root = fs.revision_root(self.fs, self.rev)
+    editor = BatonCollector(self.fs, root)
+    e_ptr, e_baton = delta.make_editor(editor)
+    repos.replay(root, e_ptr, e_baton)
+    for baton in editor.batons:
+      self.assertEqual(sys.getrefcount(baton[2]), 2,
+                       "leak on baton %s after replay without errors"
+                       % repr(baton))
+    del e_baton
+    self.assertEqual(sys.getrefcount(e_ptr), 2,
+                     "leak on editor baton after replay without errors")
+
+    editor = BatonCollectorErrorOnClose(self.fs, root,
+                                        error_path=b'branches/v1x')
+    e_ptr, e_baton = delta.make_editor(editor)
+    self.assertRaises(SubversionException, repos.replay, root, e_ptr, e_baton)
+    batons = editor.batons
+    # As svn_repos_replay calls neither close_edit callback nor abort_edit
+    # if an error has occured during processing, references of Python objects
+    # in decendant batons may live until e_baton is deleted.
+    del e_baton
+    for baton in batons:
+      self.assertEqual(sys.getrefcount(baton[2]), 2,
+                       "leak on baton %s after replay with an error"
+                       % repr(baton))
+    self.assertEqual(sys.getrefcount(e_ptr), 2,
+                     "leak on editor baton after replay with an error")
+
   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",

Modified: subversion/branches/1.14.x/subversion/bindings/swig/svn_delta.i
URL: 
http://svn.apache.org/viewvc/subversion/branches/1.14.x/subversion/bindings/swig/svn_delta.i?rev=1913135&r1=1913134&r2=1913135&view=diff
==============================================================================
--- subversion/branches/1.14.x/subversion/bindings/swig/svn_delta.i (original)
+++ subversion/branches/1.14.x/subversion/bindings/swig/svn_delta.i Fri Oct 20 
04:00:04 2023
@@ -68,8 +68,6 @@
    ### There must be a cleaner way to implement this? 
    ### Maybe follow Ruby by wrapping it where passing an editor? */
 void svn_swig_py_make_editor(const svn_delta_editor_t **editor,
-                             void **edit_baton,
-                             PyObject *py_editor,
                              apr_pool_t *pool);
 #endif
 
@@ -207,6 +205,49 @@ void _ops_get(int *num_ops, const svn_tx
 
 #ifdef SWIGPYTHON
 %pythoncode %{
+# Baton container class for editor/parse_fns3 batons and their decendants.
+class _ItemBaton:
+  def __init__(self, editor, pool, baton=None):
+    self.pool = pool if pool else libsvn.core.svn_pool_create()
+    self.baton = baton
+    self.editor = editor
+
+  def get_ancestor(self):
+    raise NotImplementedError
+
+  def make_decendant(self, pool, baton=None):
+    return _DecBaton(self, pool, baton)
+
+
+class _DecBaton(_ItemBaton):
+  def __init__(self, parent, pool, baton=None):
+    import weakref
+    _ItemBaton.__init__(self, parent.editor, pool, baton)
+    self._anc = weakref.ref(parent.get_ancestor())
+    self._anc().hold_baton(self)
+
+  def get_ancestor(self):
+    return self._anc()
+
+  def release_self(self):
+    self._anc().release_baton(self)
+
+
+class _AncBaton(_ItemBaton):
+  def __init__(self, editor, pool, baton=None):
+    _ItemBaton.__init__(self, editor, pool, baton)
+    self._dec = {}     # hold decendant batons.
+
+  def get_ancestor(self):
+    return self
+
+  def hold_baton(self, baton):
+    self._dec[id(baton)] = baton
+
+  def release_baton(self, baton):
+    del self._dec[id(baton)]
+
+
 # This function is for backwards compatibility only.
 # Use svn_txdelta_window_t.ops instead.
 svn_txdelta_window_t_ops_get = svn_txdelta_window_t._ops_get

Modified: subversion/branches/1.14.x/subversion/bindings/swig/svn_repos.i
URL: 
http://svn.apache.org/viewvc/subversion/branches/1.14.x/subversion/bindings/swig/svn_repos.i?rev=1913135&r1=1913134&r2=1913135&view=diff
==============================================================================
--- subversion/branches/1.14.x/subversion/bindings/swig/svn_repos.i (original)
+++ subversion/branches/1.14.x/subversion/bindings/swig/svn_repos.i Fri Oct 20 
04:00:04 2023
@@ -152,8 +152,6 @@ svn_error_t *svn_repos_dump_fs2(svn_repo
 #ifdef SWIGPYTHON
 /* Make swig wrap this function for us, to allow making a vtable in python */
 void svn_swig_py_make_parse_fns3(const svn_repos_parse_fns3_t **parse_fns3,
-                                 void **parse_baton,
-                                 PyObject *py_parse_fns3,
                                  apr_pool_t *pool);
 #endif
 


Reply via email to