[Python-Dev] bpo-36829: Add sys.unraisablehook()

[Nathaniel Smith]

On Thu, May 16, 2019 at 1:23 PM Victor Stinner <vstinner at redhat.com> wrote:
>
> Le jeu. 16 mai 2019 à 20:58, Petr Viktorin <encukou at gmail.com> a écrit :
> > I always thought the classic (exc_type, exc_value, exc_tb) triple is a
> > holdover from older Python versions, and all the information is now in
> > the exception instance.
> > Is the triple ever different from (type(exc), exc, exc.__traceback__)?
> > (possibly with a getattr for __traceback__)
>
> I added assertions in PyErr_WriteTraceback():
>
>     assert(Py_TYPE(v) == t);
>     assert(PyException_GetTraceback(v) == tb);
>
> "Py_TYPE(v) == t" fails in
> test_exceptions.test_memory_error_in_PyErr_PrintEx() for example.
> PyErr_NoMemory() calls PyErr_SetNone(PyExc_MemoryError), it sets
> tstate->curexc_type to PyExc_MemoryError, but tstate->curexc_value is
> set to NULLL.

This makes some sense – if you can't allocate memory, then better not
allocate an exception instance to report that! So this is legitimately
a special case.

But... it looks like everywhere else, the way we handle this when
transitioning into Python code is to create an instance. For example,
that test does 'except MemoryError as e', so an instance does need to
be created then. The comments suggest that there's some trick where we
have pre-allocated MemoryError() instances? But either way, if we can
afford to call a Python hook (which requires at least allocating a
frame!), then we can probably also afford to materialize the
MemoryError instance. So I feel like maybe we shouldn't be passing
None to the unraisable hook, even if PyErr_NoMemory() did initially
set that?

Also, in practice, the only time I've ever seen MemoryError is from
attempting to do a single massively-huge allocation. It's never meant
that regular allocation of small objects will fail.

> "PyException_GetTraceback(v) == tb" fails in
> test_exceptions.test_unraisable() for example: "PyTraceBack_Here(f);"
> in the "error:" label of ceval.c creates a traceback object and sets
> it to tstate->curexec_traceback, but it doesn't set the __traceback__
> attribute of the current exception.

Isn't this just a bug that should be fixed?

> > Should new APIs use it?
>
> I tried to add a "PyErr_NormalizeException(&t, &v, &tb);" call in
> PyErr_WriteUnraisable(): it creates an exception object (exc_value)
> for the PyErr_NoMemory() case, but it still doesn't set the
> __traceback__ attribute of the exception for the PyTraceBack_Here()
> case.
>
> It seems like PyErr_WriteUnraisable() cannot avoid having 3 variables
> (exc_type, exc_value, exc_tb), since they are not consistent as you
> may expect.

I'm actually fine with it having three arguments -- even if it's
technically unnecessary, it's currently 100% consistent across these
low-level APIs, and it doesn't hurt anything, so we might as well
stick with it for consistency.

-n

-- 
Nathaniel J. Smith -- https://vorpus.org