C API functions should never be called when the error indicator is set

[iritkatriel]

There are functions that may not be called with an error set, for example:

    /* PyObject_Repr() must not be called with an exception set,
       because it can clear it (directly or indirectly) and so the
       caller loses its exception */
    assert(!_PyErr_Occurred(tstate));

But this is not asserted for most of the C API functions.

In Python, it is not possible to execute code when there is an in-flight exception so C API should not allow that either and it should be checked for all API functions.

[vstinner]

I introduced these assertions gradually since stdlib C extensions and many third party extensions relied on the old behavior. Hopefully, user feedback was positive: people understood and these issues got fixed. I want to believe that it helped to fix some bugs but I forgot about the details. Over time, I tried to move the assertions closer to get the error earlier, and catch more bugs. Recently, I added these assertions to some type slots.

One issue is that few developers use a Python debug build. The Python developer mode doesn't help here. For example, I don't think that GitHub Actions provide a Python debug build. I made the debug buid ABI compatible with the release build to benefit of these assertions.

I proposed a few times to remove runtime checks in the C API ("bad API call") from the release build, to suggest using the debug build instead, to speed up Python. I abandonned my idea when I saw that GitHib Actions miss the Python debug build.

[davidhewitt]

Is it a rule that (unless documented) C API functions should not be called with the error indicator set? If so, do we document this rule anywhere?

With exception chaining I don't think this is strictly necessary, functions such as PyErr_SetString could be changed to create exception chains if C API calls failed while an exception is already present (or more likely replaced with alternatives). I think this would need #1 to be solved, because PyErr_Occurred could not be used to check for error returns in API functions which could be called with the error indicator set.

[vstinner]

Calling a function with an exeption set is unsafe if it can clear the current exception or raise a new one without chaining it to the previous exeption. See also my rejected https://peps.python.org/pep-0490/ which proposed to chain exceptions by default.

[iritkatriel]

Now that we have PEP-678, we could create a macro that (maybe only in debug builds) checks if an exception is set, and if so, adds a note to it about which function was called with the exception set and returns. This way exceptions won't be swallowed and the traceback will show which API functions were called with an exception set.

[iritkatriel]

Is it a rule that (unless documented) C API functions should not be called with the error indicator set? If so, do we document this rule anywhere?

With exception chaining I don't think this is strictly necessary, functions such as PyErr_SetString could be changed to create exception chains if C API calls failed while an exception is already present (or more likely replaced with alternatives).

This would create a capability in C API that does not exist in python: in Python when there is an in-flight exception your code is not being executed until that exception is caught. Why should the C API allow this?

[vstinner]

There are _Py_CheckFunctionResult() and _Py_CheckSlotResult() functions which raise a SystemError exception if the C API is misused. They chain the new SystemError exception if a function returns a result with an exception set.

I added assert(!_PyErr_Occurred(tstate)); as an assertion to ensure that the program is immediately killed with SIGABRT signal to force developers to fix their bugs. If you "only" raise a SystemError, it might be ignored with badly written try/except-like code (especially in C).

If you would prefer to give more freedom on how such bug is handled, look at the addition of sys.excepthook to Python 3.8: python/cpython#81010. I wrote Python 3.8 sys.unraisablehook article about it.

[iritkatriel]

_Py_CheckFunctionResult() and _Py_CheckSlotResult() are something else. They check at the end of a function that the error indicator and the return value are consistent. What I'm talking about here is a check at the beginning of a C API function that the error indicator is not set.

[gvanrossum]

Is the solution here that any call to a C API function while the error indicator is set is wrong (even though this is not always verified), or is the solution for anything that raises to chain an existing exception? Or something else? Is this something we can fix incrementally, or would it be too disruptive?

[iritkatriel]

I think automatic chaining here muddies the waters. In python you chain a handled exception as the error. Here we would be chaining a raised but not yet caught exception. This is not what python does or can do because it can’t execute code when there is a raised exception.

Strictly speaking I think any c api function should return null immediately if it is called with an exception set (assert in debug mode). It would break a lot of code though so I’m not sure about doing it incrementally.

[encukou]

Right, IMO calling C API with an exception set should be an error in general. (But the are exceptions to the general rule, like PyErr_ExceptionMatches, so it's not trivial to document this. #34 makes it hard to list the exceptions.)

I'm not convinced we should check everywhere (outside debug mode) -- is if (!PyErr_Occurred()) cheap enough?

Chaining the raised-but-not-caught exception should be done explicitly. Public API for it would be useful, IMO.

[vstinner]

Many C API functions are used to create an exception instance and to handle the current exception. It's not easy to draw a line between functions which can be called with an exeption set and the other ones.