bpo-41435: Add sys._current_exceptions() function #21689

jd · 2020-07-30T11:36:18Z

This adds a new function named sys._current_exceptions() which is equivalent ot
sys._current_frames() except that it returns the exceptions currently handled
by other threads. It is equivalent to calling sys.exc_info() for each running
thread.

https://bugs.python.org/issue41435

gvanrossum · 2020-08-03T00:59:56Z

This is a fantastic idea. I don't have time to review it but hopefully @serhiy-storchaka can have a look.

serhiy-storchaka

I am not sure that adding sys._current_exceptions() is the best solution.

serhiy-storchaka · 2020-08-03T13:23:09Z

Python/pystate.c

@@ -1195,6 +1195,66 @@ _PyThread_CurrentFrames(void)
    return result;
 }

+/* The implementation of sys._current_exceptions(). This is intended to be
+   called with the GIL held, as it will be when called via
+   sys._current_frames(). It's possible it would work fine even without the GIL


I have doubts that it would work fine without the GIL held because of: 1) auditing, 2) memory allocation.

I copy/pasted the comment from sys._current_frames since they use the same mechanism. I can also factor the code and the comment elsewhere.

You must not use the Python C API without holding the GIL: PyLong_FromUnsignedLong(), PyDict_SetItem(), etc. require to hold the GIL.

Ok, we might want to update the comment for _current_frames then!

Python/pystate.c

gvanrossum · 2020-08-03T16:16:18Z

@serhiy-storchaka

I am not sure that adding sys._current_exceptions() is the best solution.

What would you propose? The use case is real.

serhiy-storchaka · 2020-08-07T14:45:56Z

I do not know what the use case is exactly (I also do not know the use case of sys._current_frames()). I am wondering if adding methods or properties for getting current handled exception for specified frame or thread would be more general solution.

jd · 2020-08-07T15:21:28Z

In my case, the use-case for both functions is to be able to do statistical profiling on programs and to learn which code paths are raising the most exceptions, which type of exceptions, etc.

We do this currently in a hacky way with Cython and -DPy_BUILD_CORE: https://github.com/DataDog/dd-trace-py/blob/master/ddtrace/profiling/collector/stack.pyx#L301-L323

You can look at what Java and JFR do in this regard for example: https://stackoverflow.com/questions/60147519/how-to-profile-the-number-of-exceptions-generated-categorized-by-exception-clas

jd · 2020-08-25T10:07:46Z

@serhiy-storchaka Does that make a compelling argument? :)

jd · 2020-09-15T11:41:44Z

@vstinner any opinion on this?

vstinner · 2020-09-15T15:25:10Z

Lib/test/test_sys.py

+                try:
+                    raise ValueError("oops")
+                except ValueError:
+                    if leave_g.wait(timeout=0.1):


I would prefer to use support.SHORT_TIMEOUT or even support.LONG_TIMEOUT if the timeout should not occur.

vstinner · 2020-09-15T15:25:10Z

Python/pystate.c

+PyObject *
+_PyThread_CurrentExceptions(void)
+{
+    PyThreadState *tstate = _PyThreadState_GET();


I would prefer to ensure that the GIL is held. For example, call _Py_EnsureTstateNotNULL(tstate).

vstinner · 2020-09-15T15:25:10Z

Python/pystate.c

@@ -1195,6 +1195,66 @@ _PyThread_CurrentFrames(void)
    return result;
 }

+/* The implementation of sys._current_exceptions(). This is intended to be
+   called with the GIL held, as it will be when called via
+   sys._current_frames(). It's possible it would work fine even without the GIL


You must not use the Python C API without holding the GIL: PyLong_FromUnsignedLong(), PyDict_SetItem(), etc. require to hold the GIL.

vstinner · 2020-09-15T15:25:10Z

Python/sysmodule.c

+/*[clinic input]
+sys._current_exceptions
+
+Return a dict mapping each thread's thread id to its current raised exception.


-thread

=> thread's identifier

vstinner · 2020-09-15T15:25:10Z

Python/pystate.c

+            if (err_info == NULL) {
+                continue;
+            }
+            PyObject *id = PyLong_FromUnsignedLong(t->thread_id);


A single thread can be associated to more than one Python thread state. It is possible that two interpreters run in the same thread, just not at the same time.

This is also a limitation in _current_frames then, right?
I think it'd be fine in this case to mimic what _current_frames does in this regard.

vstinner · 2020-09-15T15:25:10Z

Doc/library/sys.rst

+.. function:: _current_exceptions()
+
+   Return a dictionary mapping each thread's identifier to the topmost exception
+   currently active in that thread at the time the function is called.


It's not clear that threads with no exception are omitted in this dictionary.

vstinner · 2020-10-06T12:44:14Z

cc @markshannon @scoder

serhiy-storchaka · 2020-10-14T09:57:52Z

Python/pystate.c

+                goto fail;
+            }
+            PyObject *exc_info = PyTuple_Pack(3,
+                                              err_info->exc_type != NULL ? err_info->exc_type : Py_None,


These lines are too long. I propose to reduce indentation,

Sure, re-indented.

This adds a new function named sys._current_exceptions() which is equivalent ot sys._current_frames() except that it returns the exceptions currently handled by other threads. It is equivalent to calling sys.exc_info() for each running thread.

jd · 2020-11-02T13:28:52Z

Can someone press the button?
cc @serhiy-storchaka @vstinner

This adds a new function named sys._current_exceptions() which is equivalent ot sys._current_frames() except that it returns the exceptions currently handled by other threads. It is equivalent to calling sys.exc_info() for each running thread.

the-knights-who-say-ni added the CLA signed label Jul 30, 2020

bedevere-bot added the awaiting review label Jul 30, 2020

gvanrossum requested a review from serhiy-storchaka Aug 3, 2020

serhiy-storchaka reviewed Aug 3, 2020

View changes

jd force-pushed the sys-current-exceptions branch from 454c7c3 to 789d9b0 Compare Aug 3, 2020

jd force-pushed the sys-current-exceptions branch from a2d698c to e21084d Compare Aug 7, 2020

vstinner reviewed Sep 15, 2020

View changes

jd force-pushed the sys-current-exceptions branch from e21084d to 62f48e2 Compare Sep 28, 2020

jd requested a review from vstinner Sep 28, 2020

jd force-pushed the sys-current-exceptions branch 2 times, most recently from eeed951 to ae0e5d7 Compare Sep 28, 2020

serhiy-storchaka approved these changes Oct 14, 2020

View changes

bedevere-bot added awaiting merge and removed awaiting review labels Oct 14, 2020

jd force-pushed the sys-current-exceptions branch from ae0e5d7 to 0726d17 Compare Oct 14, 2020

serhiy-storchaka merged commit 64366fa into python:master Nov 2, 2020
3 checks passed

bedevere-bot removed the awaiting merge label Nov 2, 2020

bpo-41435: Add sys._current_exceptions() function #21689

bpo-41435: Add sys._current_exceptions() function #21689

jd commented Jul 30, 2020 •

edited by bedevere-bot

gvanrossum commented Aug 3, 2020

serhiy-storchaka left a comment

serhiy-storchaka Aug 3, 2020

jd Aug 3, 2020

vstinner Sep 15, 2020

jd Sep 28, 2020

gvanrossum commented Aug 3, 2020

serhiy-storchaka commented Aug 7, 2020

jd commented Aug 7, 2020

jd commented Aug 25, 2020

jd commented Sep 15, 2020

vstinner Sep 15, 2020

jd Sep 28, 2020

vstinner Sep 15, 2020

jd Sep 28, 2020

vstinner Sep 15, 2020

vstinner Sep 15, 2020

jd Sep 28, 2020

vstinner Sep 15, 2020

jd Sep 28, 2020

vstinner Sep 15, 2020

jd Sep 28, 2020

vstinner commented Oct 6, 2020

serhiy-storchaka Oct 14, 2020

jd Oct 14, 2020 •

edited

jd commented Nov 2, 2020

bpo-41435: Add sys._current_exceptions() function #21689

bpo-41435: Add sys._current_exceptions() function #21689

Conversation

jd commented Jul 30, 2020 • edited by bedevere-bot

gvanrossum commented Aug 3, 2020

serhiy-storchaka left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gvanrossum commented Aug 3, 2020

serhiy-storchaka commented Aug 7, 2020

jd commented Aug 7, 2020

jd commented Aug 25, 2020

jd commented Sep 15, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vstinner commented Oct 6, 2020

Choose a reason for hiding this comment

jd Oct 14, 2020 • edited

Choose a reason for hiding this comment

jd commented Nov 2, 2020

jd commented Jul 30, 2020 •

edited by bedevere-bot

jd Oct 14, 2020 •

edited