Handle contradictory error conditions from C extensions consistently

This probably needs to be a PEP, but I thought I’d see what folks here thought before I write one.

When a C extension function succeeds it should return non-NULL and not set an exception.
When it fails, it should return NULL and set an exception.
That’s fine when C extensions behave, but what happens when they don’t?

If a C extension function returns non-NULL and sets an exception, or if it returns NULL and doesn’t set an exception, what should happen?

Currently, almost anything can happen. The interpreter might abort with a FatalError, a SystemError might be raised or, in some cases, the interpreter might crash.

I propose we fix this.
We should add a new exception type, ExtensionException and raise that any time a C extension returns inconsistent results.

Thanks to improvements to the way exceptions are handled internally in newer versions of CPython, the cost of any additional checks should be low. They might even be faster than the ad-hoc checks we currently have.

7 Likes

A correct C extension function should never do this.

True. But it still happens

3 Likes

Print an error to stderr and exit python maybe?

Do you think this is a situation where there are extensions that appear to work that break this API rule?

This sounds reasonable, especially if ExtensionException is a subclass of SystemError.
But the scope isn’t all that clear to me. What exactly do you mean by “extension function”? For example, are type slots in scope?
And when exactly should the check be done? Are callers other than CPython are expected to do it? If not, I don’t think extension authors can rely on it. Maybe this is meant to be an internal mechanism/guideline for core devs?

There are many other programming errors, such as using memory after free or access past the end of the array. This is C, it allows such kinds of errors. It is not possible to detect them reliably, but we can make debugging easier by filling the freed memory with specific pattern or adding index checks. Such methods has a non-zero cost, this is why they are usually only enabled in special builds.

I would use a mode that allows for detecting these errors in a testing/debug setup.

Could the checking and reporting be enabled by an option to python?

It is already done. Some checks are only enabled in the debug build. Others are enabled in the “development mode” (-X dev). Others need both the debug build and the “development mode”.

This could be a new check that -x dev does then?

What this?

This == checking return value consistent with exception raised,
which is subject of this topic.

You need the debug build for this. There are a lot of assertions like the following:

assert((value != NULL) == !PyErr_Occurred());
assert((err == -1) == (PyErr_Occurred() != NULL));
assert((res == NULL) != (PyErr_Occurred() == NULL));
assert((res != NULL) ^ (_PyErr_Occurred(tstate) != NULL));