The original proposition was about conditionally aborting the current process. Raising an exception (in particularly byos.exit()) does not work. Should not we add os.abort()? Or there are better alternatives for post-mortem investigation?

@vstinner PyErr_WriteUnraisable can get called during__del__ or GC so most of the python environment could have already been deleted by the time the hook is called,

Or the hook could be called after the hook is removed by GC and then we're back to the same problem

the other PR for reference:#13175

@vstinner PyErr_WriteUnraisable can get called duringdel or GC so most of the python environment could have already been deleted by the time the hook is called, Or the hook could be called after the hook is removed by GC and then we're back to the same problem

It works as expected on my tests:https://bugs.python.org/issue36829#msg341868

Should not we add os.abort()?

That sounds like a reasonable addition, expose C abort() as os.abort().

Without os.abort(), you can call abort() using "import signal; signal.raise_signal(signal.SIGABRT)". But it only works if SIGABRT signal handler has not been overriden. Or maybe you can call "signal.signal(signal.SIGABRT, signal.SIG_DFL)" to restore the original signal handler.

Or the hook could be called after the hook is removed by GC and then we're back to the same problem

My implementations uses the default builtin hook if sys.unraisablehook doesn't exist or has been set to None.

I am not sure if we can trick PyImport_Cleanup to ensure that custom hook remains alive as long as possible.

I modified my PR to log exception if custom sys.unraisablehook itself raises an exception: see the new dedicated unit test.

I'm not sure that it's safe to run arbitrary Python code any time thatPyErr_WriteUnraisable is called. There might be occasions wherePyErr_WriteUnraisable is called but where CPython is internally in an inconsistent state where calling Python code would crash the interpreter.

When a custom hook fails, the exception is logged. For example, test_custom_unraisablehook_fail() checks for "Exception: hook_func failed" in the output.

Another example:

import _testcapiimport sysdef hook_func(*args):    raise Exception("hook_func failed")sys.unraisablehook = hook_func_testcapi.write_unraisable(ValueError(42), None)

Output:

sys.unraisablehook failed:Traceback (most recent call last):  File "x.py", line 5, in hook_func    raise Exception("hook_func failed")Exception: hook_func failed

I'm not sure that it's safe to run arbitrary Python code any time that PyErr_WriteUnraisable is called. There might be occasions where PyErr_WriteUnraisable is called but where CPython is internally in an inconsistent state where calling Python code would crash the interpreter.

Do you have an example? I'm not aware of such issue.

Python must always remain consistent. Usually, Py_FatalError() is called when an inconstancy is detected.

If the hook fails, will it abort the process with a non-zero exit code?

No, the second new error is logged. Python should not be aborted when something goes wrong: it should be possible to continue the execution.

Another complete solution forhttps://bugs.python.org/issue36829 would be to add a new -X command line option to use a specific exit code if at least one "unraisable" exception is logged. So far, nobody showed an example where my hook cannot be used to fixhttps://bugs.python.org/issue36829

Seehttps://bugs.python.org/issue36829#msg342000 for PyErr_WriteUnraisable() called very late during Python shutdown.

I added "msg" to sys.unraisablehook.

Ok, so I rebased my PR on top of master to reduce the risk of conflicts, I squashed my commits to be able to update the main commit message, and I added a new "msg" parameter to sys.unraisablehook.

The new "msg" parameter allows to pass an arbitrary error message rather than the hardcoded "Exception ignored in: ". I added _PyErr_WriteUnraisableMsg() to log an unraisable exception with an error message.

I only used _PyErr_WriteUnraisableMsg() twice where it was appropriate to show how it can be used. If the PR is accepted, a following PR can be written to add a more accurate error message (than the hardcoded error message). I prefer to keep this PR as small as possible.

I also cleaned up the implementation and added more comments.

cc@methane@pablogsal

@serhiy-storchaka: Would you mind to review the updated PR? Do you like the new API?

I plan to merge this PR at the end of the week. I addressed all remarks/requests.

The only remaining complain about this PR is the fact that it doesn't allow to catch exceptions raised very late during Python shutdown:https://bugs.python.org/issue36829#msg342001

I confirm that it's a known limitation, but I am not comfortable with killing (SIGABRT) Python when PyErr_WriteUnraisable() is called late during Python finalization: there is no "easy" way to debug such issue. Only a low-level debugger like gdb can maybe provide some info about what happened. Or maybe not since Python finalization already destroyed too many things: modules, thread state, etc.

Python finalization code is very fragile and it's a work-in-progress for 5 years. I would love to see enhancements, but multiple attempts failed badly with adding new corner cases and triggering new crashes, and so had to be reverted.

My PR is a compromise for the most common cases. IMHO it's a way more generic solution than only allow to kill the process with SIGABRT. You are free to log "unraisable exceptions" into a file, raise a signal, send exceptions into a network socket, etc. Basically, whatever you want ;-)

Do you have an example? I'm not aware of such issue.

I don't know any concrete example, it was just a general comment.

Python must always remain consistent.

I meant temporary inconsistency, when you are in the middle of updating some data structure.

I meant temporary inconsistency, when you are in the middle of updating some data structure.

Ok, I see what you mean. If we have such code, it's a bug that should be fixed.

The default hook implementation, current PyErr_WriteUnraisable() implementation, already executes "arbitrary" Python code:

• call repr(exception_value) which can be an arbitrary Python function
• get exception_type.module attribute
• call sys.stderr.write() whereas sys.stderr can be a custom object
• displaying the traceback calls io.open(filename, "rb"), parse the encoding cookie, create a io.TextIOWrapper, call fp.readline() and then fp.close()
• etc.

Moreover, the GC is not disabled and can be triggered anytime. So it's important that everything remains consistent when PyErr_WriteUnraisable() is called.

OK, I agree. Like I said, it was just a comment, not a complaint.

OK, I agree. Like I said, it was just a comment, not a complaint.

Thanks for sharing your concerns. This issue is very tricky, I wouldn't be surprised if we discover a corner case tomorrow ;-) So having more people looking into the code and how it's used is very helpful!

New version of my API. sys.unraisablehook now gets a single argument which has 4 fields:

• exc_type
• exc_value
• exc_tb
• obj

It becomes possible to extend this object later to add new attribute without breaking the backward compatibility (existing hooks used in the wild).

I also reverted unrelated changes.

I removed _PyErr_WriteUnraisableMsg() and the "err_msg" parameter / field. I tried to write a PR as small as possible. Once this PR will be merged, I will work on a second PR to add a new err_msg field to the hook and add back _PyErr_WriteUnraisableMsg() function.

_PySys_GetObjectId() returns a borrowed reference.

To be clear, I trust no one, especially myself: since I started to work on this PR, I'm running frequently "./python -m test test_sys -R 3:3" to make sure that nothing leaks :-) I added multiple tests in test_sys, and it happened multiple times that I introduced a giant leak because of an obvious bug in my code :-D

@pablogsal: I addressed your comments.

I squashed my commits, rebased them on master and fix a merge conflict.

@serhiy-storchaka@pablogsal@njsmith: Would you mind to review my PR adding sys.unraisablehook?

I renamed UnraisableHookArgs fields to (exc_type, exc_value, exc_traceback, object). I also fixed a few more bugs in the implementation to fix a few more corner cases like exc_type == NULL.

I also completed the documentation.

I plan to merge this change next Wednesday (in 2 days). I didn't see any strong opinion to the API, and I'm not convinced by other proposed APIs.

Once this PR will be merged, I'm interested to work on follow-up (I plan to write 1 PR to bullet):

• Add an optional error message to UnraisableHookArgs to override default "Exception ignored in:"
• Try to normalize the exception
• Try to create a traceback object and attack it to the exception
• Add a helper function to test.support to catch unraisable exception and modify unit tests generating such unraisable exception
• Maybe also experiment to modify regrtest to log these unraisable exceptions to display them again in the summary at the end

Thanks for reviews, I merged my PR!

Follow-up:

• PR[3.7] bpo-36829: Enhance PyErr_WriteUnraisable() #13487 backports enhancement and bugfix to Python 3.7
• PRbpo-36829: Add _PyErr_WriteUnraisableMsg() #13488 adds _PyErr_WriteUnraisableMsg() and adds 'err_msg' field to sys.unraisablehook
• PRbpo-36829: Add test.support.catch_unraisable_exception() #13490 adds test.support.catch_unraisable_exception()