Who should call PyErr_Fetch?

Question

Many functions in the C API for Python are not safe to use if the error indicator might be set. In particular, PyFloat_AsDouble and similar functions are ambiguous in that they have no return value reserved for indicating an error: if they succeed (but happen to return the value used for errors), the client that calls PyErr_Occurred will believe them to have failed if the error indicator was simply already set. (Note that this is more or less guaranteed to happen with PyIter_Next.) More generally, any function which can fail overwrites the error indicator if it does, which may or may not be desirable.

Unfortunately, the possibility of calling such functions with the error indicator set is not at all unlikely: a common reaction to an error is to Py_DECREF local variables, and (unless the types of all objects that might be (indirectly) freed by it are known) that can execute arbitrary code. (This is a good example of the danger of having cleanup code with the possibility of failure.) The interpreter catches exceptions raised in such destructors, but it does not prevent exceptions from leaking into them.

At either end, we can use PyErr_Fetch and PyErr_Restore to prevent these issues. Put around a call to an ambiguous function, they allow reliably determining whether it succeeded; put around Py_DECREF, they prevent the error indicator from being set during the execution of whatever susceptible code in the first place. (They can also be used even around directly-invoked cleanup code that might fail, so as to allow choosing which exception to propagate. There’s no question about where to put it in this case: the cleanup code can’t choose between multiple exceptions anyway.)

Either choice of placement significantly increases code complexity and execution time: there are a lot of calls to ambiguous functions, and there are a lot of Py_DECREFs on error-handling paths. While the principle of defensive programming would suggest using it in both places, much nicer code would result from (careful programming with) a universal convention (to cover the arbitrary code being executed).

C itself has such a convention: errno must be saved by the caller of arbitrary code even if (like the suppressed exceptions in Python destructors) that code is not expected to set errno to anything. The main reason is that it can be reset (but never to 0) by many successful library calls (to let them handle errors internally), further narrowing the set of operations safe to perform while errno holds some significant value. (This also prevents the issue that arises when PyErr_Occurred reports on a preexisting error: C programmers must set errno to 0 before calling an ambiguous function.) Another reason is that “call some arbitrary code with no error reporting” is not a common operation in most C programs, so burdening other code for its sake would be nonsensical.

Is there such a convention (even if there is buggy code that doesn’t follow it in CPython itself)? Failing that, is there a technical reason to guide the choice of one to establish? Or maybe is this an engineering problem based on too literal a reading of “arbitrary”: should CPython save and restore the error indicator itself while it’s handling destructor exceptions anyway?

Answer 1

If your cleanup is just a bunch of Py_DECREF, you shouldn't need to call PyErr_Fetch. Py_DECREF is intended to be safe to call with an exception set. If the code inside Py_DECREF needs to do something that isn't safe to do with an exception set, it will take responsibility for saving and restoring the exception state. (If your cleanup involves more than just Py_DECREF, you may need to handle things yourself.)

For example, tp_finalize, one of the steps of object destruction most likely to invoke arbitrary Python code, is explicitly responsible for saving and restoring an active exception:

tp_finalize should not mutate the current exception status; therefore, a recommended way to write a non-trivial finalizer is:

static void
local_finalize(PyObject *self)
{
    PyObject *error_type, *error_value, *error_traceback;

    /* Save the current exception, if any. */
    PyErr_Fetch(&error_type, &error_value, &error_traceback);

    /* ... */

    /* Restore the saved exception. */
    PyErr_Restore(error_type, error_value, error_traceback);
}

For __del__ methods written in Python, you can see the relevant handling in slot_tp_finalize:

/* Save the current exception, if any. */
PyErr_Fetch(&error_type, &error_value, &error_traceback);

/* Execute __del__ method, if any. */
del = lookup_maybe_method(self, &PyId___del__, &unbound);
if (del != NULL) {
    res = call_unbound_noarg(unbound, del, self);
    if (res == NULL)
        PyErr_WriteUnraisable(del);
    else
        Py_DECREF(res);
    Py_DECREF(del);
}

/* Restore the saved exception. */
PyErr_Restore(error_type, error_value, error_traceback);

The weak reference system also takes responsibility for saving exception state before invoking weak reference callbacks:

if (*list != NULL) {
    PyWeakReference *current = *list;
    Py_ssize_t count = _PyWeakref_GetWeakrefCount(current);
    PyObject *err_type, *err_value, *err_tb;

    PyErr_Fetch(&err_type, &err_value, &err_tb);
    if (count == 1) {
        PyObject *callback = current->wr_callback;

        current->wr_callback = NULL;
        clear_weakref(current);
        if (callback != NULL) {
            if (((PyObject *)current)->ob_refcnt > 0)
                handle_callback(current, callback);
            Py_DECREF(callback);
        }
    }
    else {
        ...

So calling Py_DECREF while an exception is set is scary, and it's good that you're thinking about it, but as long as the object destruction code is behaving properly, it should be okay.

---

So what if you have to do more cleanup than just clearing your references? In that case, if your cleanup isn't safe to do with an exception set, you should probably call PyErr_Fetch, and PyErr_Restore the exception state when you're done. If something raises another exception while you're cleaning up, you can either chain it (awkward but possible at C level), or dump a short warning to stderr with PyErr_WriteUnraisable and then suppress the new exception by PyErr_Clear-ing it or by PyErr_Restore-ing the original exception state over it.