Hi,
Jun Omae told me (with patch) that exception from Python callback in
svn_swig_py_status_func2() causes segmentation fault on Python 3.7.
(The patch have already been committed as r1551888). I think cause of
segmentation fault on py3 is the change of exception handling between
py2 and py3, support for exception context.
So I looked over callbacks in swigutil_py.c, those of return value type
aren't svn_error_t *. I think they should be detached from callers
Python exception context because they can't report error to caller.
The patch attached fix them by inserting PyErr_Fetch() and PyErr_Restore()
save and restore Python error indicator.
(The patch in other thread textually conflict with this patch, though)
Cheers,
--
Yasuhito FUTATSUKI
On branch swig-py3: A follow up r1851888: save/restore Python error indicator
For all callback APIs which don't return svn_error_t * cannot notify
Python exception their caller, and as exceptions chain in Python 3,
exception conext should be detached from caller.
* subversion/bindings/swig/python/libsvn_swig_py/swigutil_py.c
(svn_swig_py_notify_func, svn_swig_py_notify_func2,
svn_swig_py_status_func, svn_swig_py_client_status_func,
svn_swig_py_status_func2, ra_callbacks_progress_func,
svn_swig_py_config_enumerator2, svn_swig_py_config_section_enumerator2):
Save error indicator before Python function call and then restore it
after call
Index: subversion/bindings/swig/python/libsvn_swig_py/swigutil_py.c
===================================================================
--- subversion/bindings/swig/python/libsvn_swig_py/swigutil_py.c
(revision 1852824)
+++ subversion/bindings/swig/python/libsvn_swig_py/swigutil_py.c
(working copy)
@@ -2767,11 +2767,18 @@
PyObject *function = baton;
PyObject *result;
svn_error_t *err = SVN_NO_ERROR;
+ PyObject *exc, *exc_type, *exc_traceback;
if (function == NULL || function == Py_None)
return;
svn_swig_py_acquire_py_lock();
+
+ /* As caller can't understand Python context and we can't notify if
+ Python call back function raise exception to caller, we must catch it
+ if it is occurred, and restore error indicator */
+ PyErr_Fetch(&exc_type, &exc, &exc_traceback);
+
if ((result = PyObject_CallFunction(function,
#if IS_PY3
(char *)"(yiiyiii)",
@@ -2796,6 +2803,9 @@
/* Our error has no place to go. :-( */
svn_error_clear(err);
+ /* Also, restore error indicator */
+ PyErr_Restore(exc_type, exc, exc_traceback);
+
svn_swig_py_release_py_lock();
}
@@ -2807,6 +2817,7 @@
PyObject *function = baton;
PyObject *result;
svn_error_t *err = SVN_NO_ERROR;
+ PyObject *exc, *exc_type, *exc_traceback;
if (function == NULL || function == Py_None)
return;
@@ -2813,6 +2824,11 @@
svn_swig_py_acquire_py_lock();
+ /* As caller can't understand Python context and we can't notify if
+ Python call back function raise exception to caller, we must catch it
+ if it is occurred, and restore error indicator */
+ PyErr_Fetch(&exc_type, &exc, &exc_traceback);
+
if ((result = PyObject_CallFunction(function,
(char *)"(O&O&)",
make_ob_wc_notify, notify,
@@ -2831,6 +2847,9 @@
/* Our error has no place to go. :-( */
svn_error_clear(err);
+ /* Also, restore error indicator */
+ PyErr_Restore(exc_type, exc, exc_traceback);
+
svn_swig_py_release_py_lock();
}
@@ -2841,11 +2860,18 @@
PyObject *function = baton;
PyObject *result;
svn_error_t *err = SVN_NO_ERROR;
+ PyObject *exc, *exc_type, *exc_traceback;
if (function == NULL || function == Py_None)
return;
svn_swig_py_acquire_py_lock();
+
+ /* As caller can't understand Python context and we can't notify if
+ Python call back function raise exception to caller, we must catch it
+ if it is occurred, and restore error indicator */
+ PyErr_Fetch(&exc_type, &exc, &exc_traceback);
+
if ((result = PyObject_CallFunction(function,
(char *)SVN_SWIG_BYTES_FMT "O&", path,
make_ob_wc_status, status)) == NULL)
@@ -2863,6 +2889,9 @@
/* Our error has no place to go. :-( */
svn_error_clear(err);
+ /* Also, restore error indicator */
+ PyErr_Restore(exc_type, exc, exc_traceback);
+
svn_swig_py_release_py_lock();
}
@@ -2874,11 +2903,18 @@
PyObject *function = baton;
PyObject *result;
svn_error_t *err = SVN_NO_ERROR;
+ PyObject *exc, *exc_type, *exc_traceback;
if (function == NULL || function == Py_None)
return;
svn_swig_py_acquire_py_lock();
+
+ /* As caller can't understand Python context and we can't notify if
+ Python call back function raise exception to caller, we must catch it
+ if it is occurred, and restore error indicator */
+ PyErr_Fetch(&exc_type, &exc, &exc_traceback);
+
if ((result = PyObject_CallFunction(function,
#if IS_PY3
(char *)"yO&O&",
@@ -2902,6 +2938,9 @@
/* Our error has no place to go. :-( */
svn_error_clear(err);
+ /* Also, restore error indicator */
+ PyErr_Restore(exc_type, exc, exc_traceback);
+
svn_swig_py_release_py_lock();
}
@@ -2965,11 +3004,18 @@
PyObject *function = baton;
PyObject *result;
svn_error_t *err = SVN_NO_ERROR;
+ PyObject *exc, *exc_type, *exc_traceback;
if (function == NULL || function == Py_None)
return;
svn_swig_py_acquire_py_lock();
+
+ /* As caller can't understand Python context and we can't notify if
+ Python call back function raise exception to caller, we must catch it
+ if it is occurred, and restore error indicator */
+ PyErr_Fetch(&exc_type, &exc, &exc_traceback);
+
if ((result = PyObject_CallFunction(function,
(char *)SVN_SWIG_BYTES_FMT "O&", path,
make_ob_wc_status, status)) == NULL)
@@ -2985,12 +3031,11 @@
}
/* Our error has no place to go. :-( */
- if (err)
- {
- svn_error_clear(err);
- PyErr_Clear();
- }
+ svn_error_clear(err);
+ /* Also, restore error indicator */
+ PyErr_Restore(exc_type, exc, exc_traceback);
+
svn_swig_py_release_py_lock();
}
@@ -4230,11 +4275,17 @@
{
PyObject *callbacks = (PyObject *)baton;
PyObject *py_callback, *py_progress, *py_total, *result;
+ PyObject *exc, *exc_type, *exc_traceback;
py_progress = py_total = NULL;
svn_swig_py_acquire_py_lock();
+ /* As caller can't understand Python context and we can't notify if
+ Python call back function raise exception to caller, we must catch it
+ if it is occurred, and restore error indicator */
+ PyErr_Fetch(&exc_type, &exc, &exc_traceback);
+
py_callback = PyObject_GetAttrString(callbacks,
(char *)"progress_func");
if (py_callback == NULL)
@@ -4274,6 +4325,9 @@
Py_XDECREF(result);
finished:
+ /* Restore error indicator */
+ PyErr_Restore(exc_type, exc, exc_traceback);
+
Py_XDECREF(py_callback);
Py_XDECREF(py_progress);
Py_XDECREF(py_total);
@@ -5134,9 +5188,15 @@
PyObject *result;
svn_error_t *err = SVN_NO_ERROR;
svn_boolean_t c_result;
+ PyObject *exc, *exc_type, *exc_traceback;
svn_swig_py_acquire_py_lock();
+ /* As caller can't understand Python context and we can't notify if
+ Python call back function raise exception to caller, we must catch it
+ if it is occurred, and restore error indicator */
+ PyErr_Fetch(&exc_type, &exc, &exc_traceback);
+
if ((result = PyObject_CallFunction(function,
#if IS_PY3
(char *)"yyO&",
@@ -5158,7 +5218,7 @@
/* Any Python exception we might have pending must be cleared,
because the SWIG wrapper will not check for it, and return a value with
the exception still set. */
- PyErr_Clear();
+ PyErr_Restore(exc_type, exc, exc_traceback);
if (err)
{
@@ -5186,9 +5246,15 @@
PyObject *result;
svn_error_t *err = SVN_NO_ERROR;
svn_boolean_t c_result;
+ PyObject *exc, *exc_type, *exc_traceback;
svn_swig_py_acquire_py_lock();
+ /* As caller can't understand Python context and we can't notify if
+ Python call back function raise exception to caller, we must catch it
+ if it is occurred, and restore error indicator */
+ PyErr_Fetch(&exc_type, &exc, &exc_traceback);
+
if ((result = PyObject_CallFunction(function,
(char *)SVN_SWIG_BYTES_FMT "O&",
name,
@@ -5205,8 +5271,9 @@
/* Any Python exception we might have pending must be cleared,
because the SWIG wrapper will not check for it, and return a value with
the exception still set. */
- PyErr_Clear();
+ PyErr_Restore(exc_type, exc, exc_traceback);
+
if (err)
{
/* We can't return the error, but let's at least stop enumeration. */
