Merge pull request #1396 from rstudio/fix/python-debug-build

.github/workflows/R-CMD-check.yaml

@@ -36,6 +36,10 @@ jobs:
           - { os: ubuntu-latest, r: 'release', python: '3.10' }
           - { os: ubuntu-latest, r: 'release', python: '3.11' }
 
+          # test with one debug build of python
+          # disabled; failing presently
+          # - { os: ubuntu-latest , r: 'release', python: 'debug' }
+
     env:
       GITHUB_PAT: ${{ secrets.GITHUB_TOKEN }}
       R_KEEP_PKG_SOURCE: yes
@@ -52,9 +56,19 @@ jobs:
           # http-user-agent: ${{ matrix.config.http-user-agent }}
 
       - uses: actions/setup-python@v4
+        if: matrix.config.python != 'debug'
         with:
           python-version: ${{ matrix.config.python }}
 
+      - if: matrix.config.python == 'debug'
+        name: setup python3-dbg
+        run: |
+          # sudo apt-get update
+          sudo apt-get install -y python3-dbg python3-pip python3-venv
+          # sudo rm -f /usr/bin/python3 /usr/bin/python
+          sudo ln -sf /usr/bin/python3-dbg /usr/bin/python3
+          sudo ln -sf /usr/bin/python3-dbg /usr/bin/python
+
       - uses: r-lib/actions/setup-r-dependencies@v2
         with:
           extra-packages: rcmdcheck remotes local::.

NEWS.md

@@ -42,6 +42,10 @@
 - Fixed an issue where `py_get_attr(silent = TRUE)` would not return an R `NULL`,
   if the attribute was missing, as documented. (#1413)
 
+- Fixed an issue where `py_get_attr(silent = TRUE)` would leave a python global
+  exception set if the attribute was missing, resulting in fatal errors when
+  running python under debug mode. (#1396)
+
 - Fixed an issue where a py capsule finalizer could access the R API from
   a background thread. (#1406)
 

src/python.cpp

@@ -2501,46 +2501,61 @@ bool py_has_attr_impl(PyObjectRef x, const std::string& name) {
   return PyObject_HasAttrString(x, name.c_str());
 }
 
-namespace {
-
-PyObjectRef py_get_common(PyObject* object,
-                          bool convert,
-                          bool silent)
-{
-  // if we have an object, return it
-  if (object != NULL)
-    return py_ref(object, convert);
-
-  // if we're silent, return new reference to Py_None
-  if (silent) {
-    Py_IncRef(Py_None);
-    return py_ref(Py_None, convert);
-  }
-
-  // otherwise, throw an R error
-  throw PythonException(py_fetch_error());
 
-}
-
-} // end anonymous namespace
 
 // [[Rcpp::export]]
 PyObjectRef py_get_attr_impl(PyObjectRef x,
                              const std::string& key,
                              bool silent = false)
 {
+  PyObject *er_type, *er_value, *er_traceback;
+  if (silent)
+    PyErr_Fetch(&er_type, &er_value, &er_traceback);
+
   PyObject* attr = PyObject_GetAttrString(x, key.c_str());
-  return py_get_common(attr, x.convert(), silent);
+
+  if(attr == NULL) { // exception was raised
+    if(silent) {
+      Py_IncRef(Py_None);
+      attr = Py_None;
+    } else
+      throw PythonException(py_fetch_error());
+  }
+
+  // must restore before calling py_ref(), which access the Python API,
+  // which will complain if the error is set.
+  if (silent)
+    PyErr_Restore(er_type, er_value, er_traceback);
+
+  return py_ref(attr, x.convert());
+
 }
 
 // [[Rcpp::export]]
 PyObjectRef py_get_item_impl(PyObjectRef x,
                              RObject key,
                              bool silent = false)
 {
+  PyObject *er_type, *er_value, *er_traceback;
+  if (silent)
+    PyErr_Fetch(&er_type, &er_value, &er_traceback);
+
   PyObjectPtr py_key(r_to_py(key, x.convert()));
   PyObject* item = PyObject_GetItem(x, py_key);
-  return py_get_common(item, x.convert(), silent);
+
+  if(item == NULL) { // exception was raised
+    if(silent) {
+      Py_IncRef(Py_None);
+      item = Py_None;
+    } else
+      throw PythonException(py_fetch_error());
+  }
+
+  // must restore before calling py_ref(), which may raise its own exception
+  if (silent)
+    PyErr_Restore(er_type, er_value, er_traceback);
+
+  return py_ref(item, x.convert());
 }
 
 // [[Rcpp::export]]