The only case I can find of strange behavior leaking out is this:

import sys

def wrap_in_sys_exit(ex):
    try:
        try:
            raise ex(-1)
        except ex as err:
            sys.exit(err)
    except BaseException as err:
        return err

print(repr(wrap_in_sys_exit(ValueError)))
print(repr(wrap_in_sys_exit(SystemExit)))

Which prints

SystemExit(ValueError(-1))
SystemExit(-1)

instead of the expected

SystemExit(ValueError(-1))
SystemExit(SystemExit(-1))

This API goes by to 1997. It is remarkable how simple and clear the code used to be:

void
PyErr_Restore(PyObject *type, PyObject *value, PyObject *traceback)
{
	PyThreadState *tstate = PyThreadState_GET();
	PyObject *oldtype, *oldvalue, *oldtraceback;

	if (traceback != NULL && !PyTraceBack_Check(traceback)) {
		/* XXX Should never happen -- fatal error instead? */
		Py_DECREF(traceback);
		traceback = NULL;
	}

	/* Save these in locals to safeguard against recursive
	   invocation through Py_XDECREF */
	oldtype = tstate->curexc_type;
	oldvalue = tstate->curexc_value;
	oldtraceback = tstate->curexc_traceback;

	tstate->curexc_type = type;
	tstate->curexc_value = value;
	tstate->curexc_traceback = traceback;

	Py_XDECREF(oldtype);
	Py_XDECREF(oldvalue);
	Py_XDECREF(oldtraceback);
}

void
PyErr_SetObject(PyObject *exception, PyObject *value)
{
	Py_XINCREF(exception);
	Py_XINCREF(value);
	PyErr_Restore(exception, value, (PyObject *)NULL);
}

The code may have been simple, but it can leave the VM in a precarious state if the value is not a legal argument for calling the type.

The new C APIs PyErr_GetRaisedException and PyErr_SetRaisedException are IMO problematic:

• the former returns a borrowed reference
• the latter steals a reference

This goes against our recommendations for adding new public C APIs.

I also find the added documentation inadequate. It is not documented that the former function returns a borrowed reference. The documentation for the latter function is not exactly friendly written, and I'm specifically thinking of the last parenthesised sentence:

exc must be a valid exception. (Violating this rules will cause subtle problems later.) This call consumes a reference to the exc object: you must own a reference to that object before the call and after the call you no longer own that reference. (If you don’t understand this, don’t use this function. I warned you.)

Suggesting to considering reopening this issue until at least the docs have been improved. IMO, we should also fix the newly added C APIs to be consistent in their reference handling, which means:

• PyErr_GetRaisedException should return a new reference
• PyErr_SetRaisedException should not steal a reference

The remark "(If you don’t understand this, don’t use this function. I warned you.)" comes directly from the documentation for PyErr_Restore and PyErr_Fetch which this API is meant to replace, as does the reference count behavior.

PyErr_GetRaisedException should return a new reference

How does that differ from the current behavior?

AFAICS, PyErr_GetRaisedException doesn't return a borrowed reference. The caller owns the return value.
It's just PyErr_SetRaisedException stealing its argument.

AFAICS, PyErr_GetRaisedException doesn't return a borrowed reference. The caller owns the return value.
It's just PyErr_SetRaisedException stealing its argument.

Huh? In that case, I'm reading this completely wrong:

Anyway, I still think it smells to introduce a new API that steals its argument. There should be no performance hit in making the caller instead of the callee having to decref that argument.

The remark "(If you don’t understand this, don’t use this function. I warned you.)" comes directly from the documentation for PyErr_Restore and PyErr_Fetch which this API is meant to replace, as does the reference count behavior.

If we introduce a new API, why copy the IMO inadequate docs and the smelly ref count behaviour? It's a new API; we can choose to follow the convention; we can choose to document it properly.

I'm not saying we shouldn't document it better, just don't blame me for the docs 🙂

I'm not saying we shouldn't document it better, just don't blame me for the docs 🙂

Sorry, if it came out that way; that was not my intent! Let's just amend the docs; I can propose a patch 🙂

(Regarding the reference counting, I'd still like to see the public APIs following the established conventions.)

(Re-opening till we've patched the docs)

OOI, is there plans for a single-arg variant of _PyErr_ChainExceptions? Or is that not needed anymore?