diff --git a/.github/workflows/R-CMD-check.yaml b/.github/workflows/R-CMD-check.yaml index 663857a58..e78d64bc8 100644 --- a/.github/workflows/R-CMD-check.yaml +++ b/.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::. diff --git a/NEWS.md b/NEWS.md index dbc98b504..d7087823e 100644 --- a/NEWS.md +++ b/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) diff --git a/src/python.cpp b/src/python.cpp index 5a325450f..56dc108b4 100644 --- a/src/python.cpp +++ b/src/python.cpp @@ -2501,36 +2501,34 @@ 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]] @@ -2538,9 +2536,26 @@ 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]]