[issue35711] Print information about an unexpectedly pending error before crashing

[STINNER Victor]

STINNER Victor  added the comment:

> A failure (crash) from such asserts provides no information on its own about 
> the unhandled exception itself to help the developer track down the C API use 
> error that led to this state.

Maybe someone should try to modify faulthandler to display the current 
exception?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35711] Print information about an unexpectedly pending error before crashing

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:

Not PyErr_Print(). Maybe PyErr_WriteUnraisable().

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35711] Print information about an unexpectedly pending error before crashing

[Gregory P. Smith]

Gregory P. Smith  added the comment:

I'm re-opening this.  The original intent of the PR was missed even though the 
change in the PR as is was not how we want to do this in the CPython VM.

Background: We build our default non-production test interpreter in `#undef 
NDEBUG` mode at Google which is why sfreilich@ ran into this while working on 
some code.

What we have today are a bunch of assert(!PyErr_Occurred()); statements around 
the CPython internals.

A failure (crash) from such asserts provides no information on its own about 
the unhandled exception itself to help the developer track down the C API use 
error that led to this state.

It'd be useful if all of these provided more information about the 
untracked/unhandled exception before crashing.

So instead of

```c
  assert(!PyErr_Occurred());
```

Something equivalent to:

```c
#ifndef NDEBUG
  if (PyErr_Occurred()) {
PyErr_Print();
assert(!PyErr_Occurred());  // crash
  }
#endif
```

were used.  But that is extremely verbose, so we wouldn't want to update all of 
the existing asserts into that.  Instead that should be encapsulated within 
something that, as with assert, compiles as a non-existant no-op in the NDEBUG 
(release) build state.  But otherwise does that.

Given this is C, a conditional macro is reasonable.  (i'd suggest a void 
function who's body is empty... but that still has consequences and would not 
be removed in all compilation and linking situations) potentially impacting 
performance.  A conditional macro similar to this would not:

```c
#ifdef NDEBUG
# define _Assert_No_PyErr_Occurred() ((void)0)
#else  /* Not NDEBUG (assertions enabled). */
# define _Assert_No_PyErr_Occurred()  do { \
int _unhandled_error = PyErr_Occurred();
if (_unhandled_error) {
PyErr_Print();
assert(!_unhandled_error);  // crash.
}
} while(0)
#endif
```

along with replacing all ~38 of our current `assert(!PyErr_Occurred());` 
statements with a call to that macro.

If, as vstinner noted in the PR comments, rather than just PyErr_Print() for 
this... using the new `_PyObject_ASSERT()` from Include/cpython/object.h macro 
giving it the unhandled exception object itself as obj would be good as it 
would produce even more useful debugging information.

Code doing that gets complicated but probably looks something like (untested):

```c
#ifdef NDEBUG
# define _Assert_No_PyErr_Occurred() ((void)0)
#else  /* Not NDEBUG (assertions enabled). */
# define _Assert_No_PyErr_Occurred()  do {  \
int _unhandled_exception = PyErr_Occurred();  \
if (_unhandled_exception) {  ]
PyObject *sys_module, *exception_object;  \
PyErr_Print();  /* Clears PyErr_Occurred() and sets sys.last_value. 
*/  \
sys_module = PyImport_ImportModule("sys");  \
assert(sys_module /* within _Assert_No_PyErr_Occurred() */);  \
exception_object = PyObject_GetAttrString(sys_module, 
"last_value");  \
if (exception_object == NULL) {  \
PyErr_Clear();  /* AttributeError */  \
/* Given PyErr_Print probably instantiated a value of there was 
 */  \
/* only a type, I don't even know if this code path is 
possible. */  \
/* Playing it safe and useful in debugging code. */  \
exception_object = PyObject_GetAttrString(sys_module, 
"last_type");  \
}  \
Py_DECREF(sys_module);  \
_PyObject_ASSERT(exception_object, _unhandled_exception);  \
/* Unreachable code, _PyObject_ASSERT should have aborted. */  \
Py_XDECREF(exception_object);  \
abort();  \
}  \
} while(0)
#endif
```

--
assignee:  -> gregory.p.smith
nosy: +gregory.p.smith
resolution: fixed -> 
status: closed -> open

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35711] Print information about an unexpectedly pending error before crashing

[STINNER Victor]

STINNER Victor  added the comment:

I close the issue, see:

https://github.com/python/cpython/pull/11513#issuecomment-454369637

--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35711] Print information about an unexpectedly pending error before crashing

[STINNER Victor]

STINNER Victor  added the comment:

I'm a supporter to add *more* checks to the debug build but also provide a 
Python runtime compiled in debug mode which would be ABI compatible (no need to 
rebuild C extensions).

See my large project:

   https://pythoncapi.readthedocs.io/

Especially:

   https://pythoncapi.readthedocs.io/runtimes.html#debug-build

It would avoid any overhead at runtime for the regular runtime (compiled in 
release mode).

Only we would have such "debug runtime" available, I even plan to remove 
runtime checks from the release build :-)

https://pythoncapi.readthedocs.io/optimization_ideas.html#remove-debug-checks

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35711] Print information about an unexpectedly pending error before crashing

[STINNER Victor]

STINNER Victor  added the comment:

I added _Py_CheckFunctionResult() to every C calls. This function calls 
PyErr_Occurred() after a function call. This change has been made in Python 
3.5: bpo-23571. I made this change because it was very difficult to identify 
bugs when an exception was raised but the bug was only spotted "later". Some 
exceptions were ignored by mistakes.

I also added a *lot* of assertions like the following one, but only when Python 
is built in debug mode:

#ifdef Py_DEBUG
/* type_call() must not be called with an exception set,
   because it may clear it (directly or indirectly) and so the
   caller loses its exception */
assert(!PyErr_Occurred());
#endif

commit 4a7cc8847276df27c8f52987cda619ca279687c2
Author: Victor Stinner 
Date:   Fri Mar 6 23:35:27 2015 +0100

Issue #23571: PyObject_Call(), PyCFunction_Call() and call_function() now
raise a SystemError if a function returns a result and raises an exception.
The SystemError is chained to the previous exception.

Refactor also PyObject_Call() and PyCFunction_Call() to make them more 
readable.

Remove some checks which became useless (duplicate checks).

Change reviewed by Serhiy Storchaka.

commit efde146b0c42f2643f96d00896c99a90d501fb69
Author: Victor Stinner 
Date:   Sat Mar 21 15:04:43 2015 +0100

Issue #23571: _Py_CheckFunctionResult() now gives the name of the function
which returned an invalid result (result+error or no result without error) 
in
the exception message.

Add also unit test to check that the exception contains the name of the
function.

Special case: the final _PyEval_EvalFrameEx() check doesn't mention the
function since it didn't execute a single function but a whole frame.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35711] Print information about an unexpectedly pending error before crashing

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:

Calling PyErr_Occurred() on every function call adds some overhead. It should 
be called only in the debug build.

In addition to the traceback, it would be nice to output also some information 
about the callable.

Note also that PyErr_Print() calls sys.excepthook and exits the interpreter if 
SystemExit is raised. It is not a proper function for using here.

--
components: +Interpreter Core
nosy: +serhiy.storchaka, vstinner
type:  -> enhancement
versions: +Python 3.8

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35711] Print information about an unexpectedly pending error before crashing

[Roundup Robot]

Change by Roundup Robot :


--
keywords: +patch, patch
pull_requests: +11071, 11072
stage:  -> patch review

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35711] Print information about an unexpectedly pending error before crashing

[Roundup Robot]

Change by Roundup Robot :


--
keywords: +patch, patch, patch
pull_requests: +11071, 11072, 11073
stage:  -> patch review

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35711] Print information about an unexpectedly pending error before crashing

[Roundup Robot]

Change by Roundup Robot :


--
keywords: +patch
pull_requests: +11071
stage:  -> patch review

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35711] Print information about an unexpectedly pending error before crashing

[Samuel Freilich]

New submission from Samuel Freilich :

_PyObject_FastCallDict and _PyObject_FastCallKeywords assert that there is no 
pending exception before calling functions that might otherwise clobber the 
exception state. However, that doesn't produce very clear output for debugging, 
since the assert failure doesn't say anything about what the pending exception 
actually was. It would be better to print the pending exception first.

--
messages: 333418
nosy: Samuel Freilich
priority: normal
severity: normal
status: open
title: Print information about an unexpectedly pending error before crashing

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com