[issue36829] CLI option to make PyErr_WriteUnraisable abort the current process

[STINNER Victor]

STINNER Victor  added the comment:

Another example of hook: hook_file.patch logs unraisable exception into 
~/unraisable.txt. Patch written for my latest PR 13187 (with the new 'msg' 
parameter).

Example of output when running the Python test suite (using multiple processes! 
./python -m test -j0 -r):
--

Exception ignored in: .C.__del__ at 
0x7f0d5c71eb00>
ZeroDivisionError: division by zero
Traceback (most recent call last):
  ...
  File "/home/vstinner/prog/python/master/Lib/test/test_raise.py", line 463, in 
test_3611
f()
  File "/home/vstinner/prog/python/master/Lib/test/test_raise.py", line 456, in 
f
del x

Exception ignored in: 
RuntimeError: generator ignored GeneratorExit
Traceback (most recent call last):
  ...
  File "/home/vstinner/prog/python/master/Lib/test/test_generators.py", line 
2207, in test_main
support.run_doctest(test_generators, verbose)
  ...
  File "", line 1, in 

del g

Exception ignored in: 
ValueError: I/O operation on closed file.
Traceback (most recent call last):
  ...
  File "/home/vstinner/prog/python/master/Lib/test/test_urllib.py", line 421, 
in test_invalid_redirect
urlopen("http://python.org/;)
  File "/home/vstinner/prog/python/master/Lib/unittest/case.py", line 237, in 
__exit__
traceback.clear_frames(tb)
  File "/home/vstinner/prog/python/master/Lib/traceback.py", line 220, in 
clear_frames
tb.tb_frame.clear()
--

IMHO such hook is more convenient than killing the process with SIGABRT ;-)

--
Added file: https://bugs.python.org/file48329/hook_file.patch

___
Python tracker 

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

[issue36829] CLI option to make PyErr_WriteUnraisable abort the current process

[STINNER Victor]

STINNER Victor  added the comment:

> Defaulting to noop when the hook fails means I'll never notice the failure to 
> be able to debug it

I'm not sure that I understood what you mean here. My PR 13187 logs a message 
into stderr is custom hook fails with a new exception. Well, then you have to 
fix your hook ;-)

--

___
Python tracker 

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

[issue36829] CLI option to make PyErr_WriteUnraisable abort the current process

[Thomas Grainger]

Thomas Grainger  added the comment:

> I'm not comfortable with forcing users to use a low-level debugger to
debug "unraisable exceptions".

Defaulting to noop when the hook fails means I'll never notice the failure
to be able to debug it

On Fri, 10 May 2019, 00:42 STINNER Victor,  wrote:

>
> STINNER Victor  added the comment:
>
> > The point for me is that CI will fail if it happens, then I can use gdb
> to find out the cause
>
> I'm not comfortable with forcing users to use a low-level debugger to
> debug "unraisable exceptions".
>
> I tried PR 13175 on the test suite by forcing the option to 1: always call
> Py_FatalError(). Many tests break:
>
> 14 tests failed:
> test_asyncio test_cmd_line test_coroutines test_cprofile
> test_exceptions test_generators test_import test_io
> test_raise test_repl test_signal test_ssl test_urllib
> test_yield_from
>
> Examples:
>
> test_error_through_destructor (test.test_io.CBufferedReaderTest) ... Fatal
> Python error: Unraisable exception
>
> FAIL: test_warn_on_full_buffer (test.test_signal.WakeupSocketSignalTests)
> FAIL: test_send_error (test.test_signal.WakeupSocketSignalTests)
> FAIL: test_wakeup_write_error (test.test_signal.WakeupSignalTests)
>
> test_unraisable (test.test_exceptions.ExceptionTests) ... Fatal Python
> error: Unraisable exception
>
> test_generators:
>
> Trying:
> del g
> Expecting nothing
> Fatal Python error: Unraisable exception
>
> etc.
>
> Unraisable exceptions are bad, but it's really hard to fix all of them.
> They are too many cases where Python is unable to pass exceptions to the
> parent.
>
> If you want to make the situation better, maybe we should investigate
> where Python cannot raise exceptions and try to make it possible.
>
> IMHO my PR 13187 adding sys.unraisablehook() is more usable/reasonable
> option.
>
> --
>
> Thomas Grainger: which code are you running on your CI? Did you try PR
> 13175 on your CI? Try it with this additional change, to always crash on
> PyErr_WriteUnraisable(). Does your CI still pass?
>
> diff --git a/Include/cpython/coreconfig.h b/Include/cpython/coreconfig.h
> index 375c0b641d..a775584553 100644
> --- a/Include/cpython/coreconfig.h
> +++ b/Include/cpython/coreconfig.h
> @@ -426,7 +426,8 @@ typedef struct {
>  .buffered_stdio = -1, \
>  ._install_importlib = 1, \
>  .check_hash_pycs_mode = NULL, \
> -._frozen = -1}
> +._frozen = -1, \
> +.abort_unraisable = 1}
>  /* Note: _PyCoreConfig_INIT sets other fields to 0/NULL */
>
>  #ifdef __cplusplus
>
> --
>
> ___
> Python tracker 
> 
> ___
>

--

___
Python tracker 

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

[issue36829] CLI option to make PyErr_WriteUnraisable abort the current process

[STINNER Victor]

STINNER Victor  added the comment:

> The point for me is that CI will fail if it happens, then I can use gdb to 
> find out the cause

I'm not comfortable with forcing users to use a low-level debugger to debug 
"unraisable exceptions".

I tried PR 13175 on the test suite by forcing the option to 1: always call 
Py_FatalError(). Many tests break:

14 tests failed:
test_asyncio test_cmd_line test_coroutines test_cprofile
test_exceptions test_generators test_import test_io
test_raise test_repl test_signal test_ssl test_urllib
test_yield_from

Examples:

test_error_through_destructor (test.test_io.CBufferedReaderTest) ... Fatal 
Python error: Unraisable exception

FAIL: test_warn_on_full_buffer (test.test_signal.WakeupSocketSignalTests)
FAIL: test_send_error (test.test_signal.WakeupSocketSignalTests)
FAIL: test_wakeup_write_error (test.test_signal.WakeupSignalTests)

test_unraisable (test.test_exceptions.ExceptionTests) ... Fatal Python error: 
Unraisable exception

test_generators:

Trying:
del g
Expecting nothing
Fatal Python error: Unraisable exception

etc.

Unraisable exceptions are bad, but it's really hard to fix all of them. They 
are too many cases where Python is unable to pass exceptions to the parent.

If you want to make the situation better, maybe we should investigate where 
Python cannot raise exceptions and try to make it possible.

IMHO my PR 13187 adding sys.unraisablehook() is more usable/reasonable option.

--

Thomas Grainger: which code are you running on your CI? Did you try PR 13175 on 
your CI? Try it with this additional change, to always crash on 
PyErr_WriteUnraisable(). Does your CI still pass?

diff --git a/Include/cpython/coreconfig.h b/Include/cpython/coreconfig.h
index 375c0b641d..a775584553 100644
--- a/Include/cpython/coreconfig.h
+++ b/Include/cpython/coreconfig.h
@@ -426,7 +426,8 @@ typedef struct {
 .buffered_stdio = -1, \
 ._install_importlib = 1, \
 .check_hash_pycs_mode = NULL, \
-._frozen = -1}
+._frozen = -1, \
+.abort_unraisable = 1}
 /* Note: _PyCoreConfig_INIT sets other fields to 0/NULL */
 
 #ifdef __cplusplus

--

___
Python tracker 

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

[issue36829] CLI option to make PyErr_WriteUnraisable abort the current process

[Thomas Grainger]

Thomas Grainger  added the comment:

The point for me is that CI will fail if it happens, then I can use gdb to
find out the cause

On Thu, 9 May 2019, 23:17 STINNER Victor,  wrote:

>
> STINNER Victor  added the comment:
>
> Ok, let me come back to the initial issue:
>
> Thomas Grainger:
> > Currently it's quite easy for these errors to go unnoticed. I'd like a
> way to easily detect these in CI. nedbat suggested piping the process
> output to another tool, and looking for 'Exception ignored in:' but this
> seems a little diff
>
> When PyErr_WriteUnraisable() is called before Python finalization, my PR
> 13187 allows to handle these exceptions: log them in a dedicated file,
> abort the process, maybe even open a network connection, etc. The hook
> allows to implement your chosen behavior.
>
> The problem is more during Python finalization: see attached
> too_late_unraisable.py example and my previous comment. If
> PyErr_WriteUnraisable() is called after sys.stderr is closed or closed to
> None, the function does nothing: the exception is not logged.
>
> The question now becomes: do *all* calls to PyErr_WriteUnraisable() must
> abort the process? What is the point? Only a very low level debugger like
> gdb can be used to see the exception.
>
> @Thomas Grainger: Do you want to have to use gdb to trace such very level
> exception?
>
> IMHO sadly when PyErr_WriteUnraisable() is called way too late, we should
> simply ignore such exceptions. And so my PR 13187 is good enough to cover
> most cases.
>
> If someone cares about exceptions raised very late during Python
> finalization, Python finalization should be enhanced. But this code is very
> fragile and is not deterministic. It is a work in progress for years to
> enhance it.
>
> --
>
> ___
> Python tracker 
> 
> ___
>

--

___
Python tracker 

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

[issue36829] CLI option to make PyErr_WriteUnraisable abort the current process

[STINNER Victor]

STINNER Victor  added the comment:

Ok, let me come back to the initial issue:

Thomas Grainger:
> Currently it's quite easy for these errors to go unnoticed. I'd like a way to 
> easily detect these in CI. nedbat suggested piping the process output to 
> another tool, and looking for 'Exception ignored in:' but this seems a little 
> diff

When PyErr_WriteUnraisable() is called before Python finalization, my PR 13187 
allows to handle these exceptions: log them in a dedicated file, abort the 
process, maybe even open a network connection, etc. The hook allows to 
implement your chosen behavior.

The problem is more during Python finalization: see attached 
too_late_unraisable.py example and my previous comment. If 
PyErr_WriteUnraisable() is called after sys.stderr is closed or closed to None, 
the function does nothing: the exception is not logged.

The question now becomes: do *all* calls to PyErr_WriteUnraisable() must abort 
the process? What is the point? Only a very low level debugger like gdb can be 
used to see the exception.

@Thomas Grainger: Do you want to have to use gdb to trace such very level 
exception?

IMHO sadly when PyErr_WriteUnraisable() is called way too late, we should 
simply ignore such exceptions. And so my PR 13187 is good enough to cover most 
cases.

If someone cares about exceptions raised very late during Python finalization, 
Python finalization should be enhanced. But this code is very fragile and is 
not deterministic. It is a work in progress for years to enhance it.

--

___
Python tracker 

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

[issue36829] CLI option to make PyErr_WriteUnraisable abort the current process

[STINNER Victor]

STINNER Victor  added the comment:

too_late_unraisable.py is an example where PyErr_WriteUnraisable() is called 
very lated during Python finalization to be handled by user code. A destructor 
(__del__) fails on print() because the stream has been closed.

PyErr_WriteUnraisable() is called by _PyGC_CollectNoFail() at the *end* of 
PyImport_Cleanup(). At this point, the sys module has already been cleared, as 
all other modules. A custom sys.unraisablehook cannot be used, because sys has 
been called.

--
versions: +Python 3.8
Added file: https://bugs.python.org/file48321/too_late_unraisable.py

___
Python tracker 

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

[issue36829] CLI option to make PyErr_WriteUnraisable abort the current process

[STINNER Victor]

STINNER Victor  added the comment:

I wrote PR 13187 to control how unraisable exceptions are handled.

Attached uncollectable.py, gc_callback.py and io_destructor.py examples can be
used to test unraisable exceptions.

Without my PR:
---
$ ./python -Werror uncollectable.py
ResourceWarning: gc: 2 uncollectable objects at shutdown; use 
gc.set_debug(gc.DEBUG_UNCOLLECTABLE) to list them

$ ./python gc_callback.py
Exception ignored in: 
Traceback (most recent call last):
  File "gc_callback.py", line 7, in wr_callback
raise ValueError(42)
ValueError: 42

$ ./python -X dev io_destructor.py
io_destructor.py:4: ResourceWarning: unclosed file <_io.TextIOWrapper 
name='io_destructor.py' mode='r' encoding='UTF-8'>
  f = None
ResourceWarning: Enable tracemalloc to get the object allocation traceback
Exception ignored in: <_io.TextIOWrapper name='io_destructor.py' mode='r' 
encoding='UTF-8'>
OSError: [Errno 9] Bad file descriptor
---

For example, apply attached site_hook.patch to install a custom unraisablehook.

Output with my PR + site_hook.patch:
---
$ ./python -Werror uncollectable.py
ResourceWarning: gc: 2 uncollectable objects at shutdown; use 
gc.set_debug(gc.DEBUG_UNCOLLECTABLE) to list them

$ ./python gc_callback.py
Exception ignored in: 
  File "gc_callback.py", line 7, in wr_callback
raise ValueError(42)
ValueError: 42
Traceback (most recent call last):
  File "gc_callback.py", line 11, in 
obj = None

$ ./python -X dev io_destructor.py
io_destructor.py:4: ResourceWarning: unclosed file <_io.TextIOWrapper 
name='io_destructor.py' mode='r' encoding='UTF-8'>
  f = None
ResourceWarning: Enable tracemalloc to get the object allocation traceback
Exception ignored in: <_io.TextIOWrapper name='io_destructor.py' mode='r' 
encoding='UTF-8'>
OSError: [Errno 9] Bad file descriptor
Traceback (most recent call last):
  File "io_destructor.py", line 4, in 
f = None
---

The first good news is that it *is* possible to write a custom hook for 
unraisable for one of the last unraisable exception: _PyGC_DumpShutdownStats() 
which logs "uncollectable objects at shutdown".

When an unraisable exceptions is logged before Python finalization, the hook 
can inspect the Python stack to see where the exception has been raised which 
helps debugging.

--

___
Python tracker 

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

[issue36829] CLI option to make PyErr_WriteUnraisable abort the current process

[STINNER Victor]

Change by STINNER Victor :


Added file: https://bugs.python.org/file48315/io_destructor.py

___
Python tracker 

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

[issue36829] CLI option to make PyErr_WriteUnraisable abort the current process

[STINNER Victor]

Change by STINNER Victor :


Added file: https://bugs.python.org/file48316/site_hook.patch

___
Python tracker 

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

[issue36829] CLI option to make PyErr_WriteUnraisable abort the current process

[STINNER Victor]

Change by STINNER Victor :


Added file: https://bugs.python.org/file48314/uncollectable.py

___
Python tracker 

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

[issue36829] CLI option to make PyErr_WriteUnraisable abort the current process

[STINNER Victor]

Change by STINNER Victor :


Added file: https://bugs.python.org/file48313/gc_callback.py

___
Python tracker 

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

[issue36829] CLI option to make PyErr_WriteUnraisable abort the current process

[STINNER Victor]

Change by STINNER Victor :


--
pull_requests: +13101

___
Python tracker 

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

[issue36829] CLI option to make PyErr_WriteUnraisable abort the current process

[Zackery Spytz]

Change by Zackery Spytz :


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

___
Python tracker 

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

[issue36829] CLI option to make PyErr_WriteUnraisable abort the current process

[Zackery Spytz]

Zackery Spytz  added the comment:

I am working on this issue.

--
nosy: +ZackerySpytz
title: CLI option to make PyErr_WriteUnraisable abortthe current process -> CLI 
option to make PyErr_WriteUnraisable abort the current process

___
Python tracker 

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