From 091dc24c365786131bbe056287a7dae1219fccd5 Mon Sep 17 00:00:00 2001 From: "Nathaniel J. Smith" Date: Fri, 19 Jan 2018 20:47:25 -0800 Subject: [PATCH 1/9] Add coro.cr_origin and sys.set_coroutine_origin_tracking_depth --- Doc/library/inspect.rst | 7 ++++++ Doc/library/sys.rst | 21 ++++++++++++++++ Include/ceval.h | 1 + Include/genobject.h | 1 + Include/pystate.h | 2 ++ Lib/test/test_coroutines.py | 49 +++++++++++++++++++++++++++++++++++-- Objects/genobject.c | 47 ++++++++++++++++++++++++++++++++++- Python/ceval.c | 9 +++++++ Python/clinic/sysmodule.c.h | 43 ++++++++++++++++++++++++++++++++ Python/pystate.c | 2 ++ Python/sysmodule.c | 31 +++++++++++++++++++++++ 11 files changed, 210 insertions(+), 3 deletions(-) create mode 100644 Python/clinic/sysmodule.c.h diff --git a/Doc/library/inspect.rst b/Doc/library/inspect.rst index 6be28a2b31cb66..2e540f467049ba 100644 --- a/Doc/library/inspect.rst +++ b/Doc/library/inspect.rst @@ -215,6 +215,10 @@ attributes: +-----------+-------------------+---------------------------+ | | cr_code | code | +-----------+-------------------+---------------------------+ +| | cr_origin | where coroutine was | +| | | created, if coroutine | +| | | origin tracking is enabled| ++-----------+-------------------+---------------------------+ | builtin | __doc__ | documentation string | +-----------+-------------------+---------------------------+ | | __name__ | original name of this | @@ -234,6 +238,9 @@ attributes: The ``__name__`` attribute of generators is now set from the function name, instead of the code name, and it can now be modified. +.. versionchanged:: 3.7 + + Add ``cr_origin`` attribute to coroutines. .. function:: getmembers(object[, predicate]) diff --git a/Doc/library/sys.rst b/Doc/library/sys.rst index 957d02b2a304f2..88e9fcf9e42e11 100644 --- a/Doc/library/sys.rst +++ b/Doc/library/sys.rst @@ -1212,6 +1212,27 @@ always available. This function has been added on a provisional basis (see :pep:`411` for details.) +.. function:: set_coroutine_origin_tracking_depth(depth) + + Allows enabling or disabling coroutine origin tracking. When + enabled, the ``cr_origin`` attribute on coroutine objects will + contain a list of (filename, line number, function name) tuples + describing the traceback where the coroutine object was created. + When disabled, ``cr_origin`` will be None. + + To enable, pass a *depth* value greater than zero; this sets the + number of frames whose information will be captured. To disable, + pass set *depth* to zero. + + Returns the old value of *depth*. + + This setting is thread-local. + + .. versionadded:: 3.7 + + .. note:: + This function has been added on a provisional basis (see :pep:`411` + for details.) Use it only for debugging purposes. .. function:: set_coroutine_wrapper(wrapper) diff --git a/Include/ceval.h b/Include/ceval.h index 70306b8bbd801a..4066132b931d9e 100644 --- a/Include/ceval.h +++ b/Include/ceval.h @@ -31,6 +31,7 @@ PyAPI_FUNC(PyObject *) PyEval_CallMethod(PyObject *obj, #ifndef Py_LIMITED_API PyAPI_FUNC(void) PyEval_SetProfile(Py_tracefunc, PyObject *); PyAPI_FUNC(void) PyEval_SetTrace(Py_tracefunc, PyObject *); +PyAPI_FUNC(int) _PyEval_SetCoroutineOriginTrackingDepth(int new_depth); PyAPI_FUNC(void) _PyEval_SetCoroutineWrapper(PyObject *); PyAPI_FUNC(PyObject *) _PyEval_GetCoroutineWrapper(void); PyAPI_FUNC(void) _PyEval_SetAsyncGenFirstiter(PyObject *); diff --git a/Include/genobject.h b/Include/genobject.h index 87fbe17d4abc03..16b983339cc649 100644 --- a/Include/genobject.h +++ b/Include/genobject.h @@ -51,6 +51,7 @@ PyAPI_FUNC(void) _PyGen_Finalize(PyObject *self); #ifndef Py_LIMITED_API typedef struct { _PyGenObject_HEAD(cr) + PyObject *cr_origin; } PyCoroObject; PyAPI_DATA(PyTypeObject) PyCoro_Type; diff --git a/Include/pystate.h b/Include/pystate.h index a4815286a74883..5a69e1471a0b9b 100644 --- a/Include/pystate.h +++ b/Include/pystate.h @@ -262,6 +262,8 @@ typedef struct _ts { void (*on_delete)(void *); void *on_delete_data; + int coroutine_origin_tracking_depth; + PyObject *coroutine_wrapper; int in_coroutine_wrapper; diff --git a/Lib/test/test_coroutines.py b/Lib/test/test_coroutines.py index d7d38a3bf9dd8b..8ada650a636b7b 100644 --- a/Lib/test/test_coroutines.py +++ b/Lib/test/test_coroutines.py @@ -1,4 +1,4 @@ -import contextlib +from contextlib import contextmanager, closing import copy import inspect import pickle @@ -58,7 +58,7 @@ def run_async__await__(coro): return buffer, result -@contextlib.contextmanager +@contextmanager def silence_coro_gc(): with warnings.catch_warnings(): warnings.simplefilter("ignore") @@ -2041,6 +2041,51 @@ def wrap(gen): sys.set_coroutine_wrapper(None) +class OriginTrackingTest(unittest.TestCase): + def here(self): + info = inspect.getframeinfo(inspect.currentframe().f_back) + return (info.filename, info.lineno) + + def test_origin_tracking(self): + orig_depth = sys.set_coroutine_origin_tracking_depth(0) + try: + async def corofn(): + pass + + with closing(corofn()) as coro: + self.assertIsNone(coro.cr_origin) + + self.assertEqual(sys.set_coroutine_origin_tracking_depth(1), 0) + + fname, lineno = self.here() + with closing(corofn()) as coro: + self.assertEqual(coro.cr_origin, + [(fname, lineno + 1, "test_origin_tracking")]) + + self.assertEqual(sys.set_coroutine_origin_tracking_depth(2), 1) + def nested(): + return (self.here(), corofn()) + fname, lineno = self.here() + ((nested_fname, nested_lineno), coro) = nested() + with closing(coro): + self.assertEqual(coro.cr_origin, + [(nested_fname, nested_lineno, "nested"), + (fname, lineno + 1, "test_origin_tracking")]) + + # Check we handle running out of frames correctly + sys.set_coroutine_origin_tracking_depth(1000) + with closing(corofn()) as coro: + self.assertTrue(2 < len(coro.cr_origin) < 1000) + + # We can't set depth negative + with self.assertRaises(ValueError): + sys.set_coroutine_origin_tracking_depth(-1) + # And trying leaves it unchanged + self.assertEqual(sys.set_coroutine_origin_tracking_depth(0), 1000) + + finally: + sys.set_coroutine_origin_tracking_depth(orig_depth) + @support.cpython_only class CAPITest(unittest.TestCase): diff --git a/Objects/genobject.c b/Objects/genobject.c index 00a882379fcabe..0a5dc7f50da1b6 100644 --- a/Objects/genobject.c +++ b/Objects/genobject.c @@ -32,6 +32,8 @@ gen_traverse(PyGenObject *gen, visitproc visit, void *arg) Py_VISIT(gen->gi_code); Py_VISIT(gen->gi_name); Py_VISIT(gen->gi_qualname); + if (((PyCodeObject *)gen->gi_code)->co_flags & CO_COROUTINE) + Py_VISIT(((PyCoroObject *)gen)->cr_origin); return exc_state_traverse(&gen->gi_exc_state, visit, arg); } @@ -137,6 +139,8 @@ gen_dealloc(PyGenObject *gen) gen->gi_frame->f_gen = NULL; Py_CLEAR(gen->gi_frame); } + if (((PyCodeObject *)gen->gi_code)->co_flags & CO_COROUTINE) + Py_CLEAR(((PyCoroObject *)gen)->cr_origin); Py_CLEAR(gen->gi_code); Py_CLEAR(gen->gi_name); Py_CLEAR(gen->gi_qualname); @@ -990,6 +994,7 @@ static PyMemberDef coro_memberlist[] = { {"cr_frame", T_OBJECT, offsetof(PyCoroObject, cr_frame), READONLY}, {"cr_running", T_BOOL, offsetof(PyCoroObject, cr_running), READONLY}, {"cr_code", T_OBJECT, offsetof(PyCoroObject, cr_code), READONLY}, + {"cr_origin", T_OBJECT, offsetof(PyCoroObject, cr_origin), READONLY}, {NULL} /* Sentinel */ }; @@ -1161,7 +1166,47 @@ PyTypeObject _PyCoroWrapper_Type = { PyObject * PyCoro_New(PyFrameObject *f, PyObject *name, PyObject *qualname) { - return gen_new_with_qualname(&PyCoro_Type, f, name, qualname); + PyObject *coro = gen_new_with_qualname(&PyCoro_Type, f, name, qualname); + if (!coro) + return NULL; + + PyThreadState *tstate = PyThreadState_GET(); + int depth = tstate->coroutine_origin_tracking_depth; + + if (depth == 0) { + ((PyCoroObject *)coro)->cr_origin = NULL; + } else { + PyObject *origin = PyList_New(depth); + /* Immediately pass ownership to coro, so on error paths we don't have + to worry about it separately. */ + ((PyCoroObject *)coro)->cr_origin = origin; + PyFrameObject *frame = PyEval_GetFrame(); + int i = 0; + for (; i < depth; ++i) { + if (!frame) + break; + + PyObject *frameinfo = Py_BuildValue( + "OiO", + frame->f_code->co_filename, + PyFrame_GetLineNumber(frame), + frame->f_code->co_name); + if (!frameinfo) { + Py_DECREF(coro); + return NULL; + } + PyList_SET_ITEM(origin, i, frameinfo); + + frame = frame->f_back; + } + /* Truncate the list if necessary */ + if (PyList_SetSlice(origin, i, depth, NULL) < 0) { + Py_DECREF(coro); + return NULL; + } + } + + return coro; } diff --git a/Python/ceval.c b/Python/ceval.c index 9276755f0d16cb..aecba34b8dbce8 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -4387,6 +4387,15 @@ PyEval_SetTrace(Py_tracefunc func, PyObject *arg) || (tstate->c_profilefunc != NULL)); } +int +_PyEval_SetCoroutineOriginTrackingDepth(int new_depth) +{ + PyThreadState *tstate = PyThreadState_GET(); + int old_depth = tstate->coroutine_origin_tracking_depth; + tstate->coroutine_origin_tracking_depth = new_depth; + return old_depth; +} + void _PyEval_SetCoroutineWrapper(PyObject *wrapper) { diff --git a/Python/clinic/sysmodule.c.h b/Python/clinic/sysmodule.c.h new file mode 100644 index 00000000000000..b68b699d05810b --- /dev/null +++ b/Python/clinic/sysmodule.c.h @@ -0,0 +1,43 @@ +/*[clinic input] +preserve +[clinic start generated code]*/ + +PyDoc_STRVAR(sys_set_coroutine_origin_tracking_depth__doc__, +"set_coroutine_origin_tracking_depth($module, /, depth)\n" +"--\n" +"\n" +"Enable or disable origin tracking for coroutine objects in this thread.\n" +"\n" +"Coroutine objects will track \'depth\' frames of traceback information about\n" +"where they came from, available in their cr_origin attribute. Set depth of 0\n" +"to disable. Returns old value."); + +#define SYS_SET_COROUTINE_ORIGIN_TRACKING_DEPTH_METHODDEF \ + {"set_coroutine_origin_tracking_depth", (PyCFunction)sys_set_coroutine_origin_tracking_depth, METH_FASTCALL|METH_KEYWORDS, sys_set_coroutine_origin_tracking_depth__doc__}, + +static int +sys_set_coroutine_origin_tracking_depth_impl(PyObject *module, int depth); + +static PyObject * +sys_set_coroutine_origin_tracking_depth(PyObject *module, PyObject *const *args, Py_ssize_t nargs, PyObject *kwnames) +{ + PyObject *return_value = NULL; + static const char * const _keywords[] = {"depth", NULL}; + static _PyArg_Parser _parser = {"i:set_coroutine_origin_tracking_depth", _keywords, 0}; + int depth; + int _return_value; + + if (!_PyArg_ParseStackAndKeywords(args, nargs, kwnames, &_parser, + &depth)) { + goto exit; + } + _return_value = sys_set_coroutine_origin_tracking_depth_impl(module, depth); + if ((_return_value == -1) && PyErr_Occurred()) { + goto exit; + } + return_value = PyLong_FromLong((long)_return_value); + +exit: + return return_value; +} +/*[clinic end generated code: output=9448b0765e4d5bf0 input=a9049054013a1b77]*/ diff --git a/Python/pystate.c b/Python/pystate.c index 028292e42fbc11..9c25a26460ea43 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -305,6 +305,8 @@ new_threadstate(PyInterpreterState *interp, int init) tstate->on_delete = NULL; tstate->on_delete_data = NULL; + tstate->coroutine_origin_tracking_depth = 0; + tstate->coroutine_wrapper = NULL; tstate->in_coroutine_wrapper = 0; diff --git a/Python/sysmodule.c b/Python/sysmodule.c index c0542436fdf1fc..6c868b8b5e880c 100644 --- a/Python/sysmodule.c +++ b/Python/sysmodule.c @@ -34,6 +34,13 @@ extern void *PyWin_DLLhModule; extern const char *PyWin_DLLVersionString; #endif +/*[clinic input] +module sys +[clinic start generated code]*/ +/*[clinic end generated code: output=da39a3ee5e6b4b0d input=3726b388feee8cea]*/ + +#include "clinic/sysmodule.c.h" + _Py_IDENTIFIER(_); _Py_IDENTIFIER(__sizeof__); _Py_IDENTIFIER(_xoptions); @@ -710,6 +717,29 @@ sys_setrecursionlimit(PyObject *self, PyObject *args) Py_RETURN_NONE; } +/*[clinic input] +sys.set_coroutine_origin_tracking_depth -> int + + depth: int + +Enable or disable origin tracking for coroutine objects in this thread. + +Coroutine objects will track 'depth' frames of traceback information about +where they came from, available in their cr_origin attribute. Set depth of 0 +to disable. Returns old value. +[clinic start generated code]*/ + +static int +sys_set_coroutine_origin_tracking_depth_impl(PyObject *module, int depth) +/*[clinic end generated code: output=1d421106d83a2fce input=f0a48609fc463a5c]*/ +{ + if (depth < 0) { + PyErr_SetString(PyExc_ValueError, "depth must be >= 0"); + return -1; + } + return _PyEval_SetCoroutineOriginTrackingDepth(depth); +} + static PyObject * sys_set_coroutine_wrapper(PyObject *self, PyObject *wrapper) { @@ -1512,6 +1542,7 @@ static PyMethodDef sys_methods[] = { {"call_tracing", sys_call_tracing, METH_VARARGS, call_tracing_doc}, {"_debugmallocstats", sys_debugmallocstats, METH_NOARGS, debugmallocstats_doc}, + SYS_SET_COROUTINE_ORIGIN_TRACKING_DEPTH_METHODDEF {"set_coroutine_wrapper", sys_set_coroutine_wrapper, METH_O, set_coroutine_wrapper_doc}, {"get_coroutine_wrapper", sys_get_coroutine_wrapper, METH_NOARGS, From d504810f43ec842c8f34d489f9199cd73d7f6eba Mon Sep 17 00:00:00 2001 From: "Nathaniel J. Smith" Date: Fri, 19 Jan 2018 23:08:23 -0800 Subject: [PATCH 2/9] Use coroutine origin information in the unawaited coroutine warning --- Include/warnings.h | 4 ++ Lib/test/test_coroutines.py | 74 +++++++++++++++++++++++++++++++++++++ Lib/warnings.py | 17 +++++++++ Objects/genobject.c | 7 +--- Python/_warnings.c | 46 +++++++++++++++++++++++ 5 files changed, 143 insertions(+), 5 deletions(-) diff --git a/Include/warnings.h b/Include/warnings.h index a3f83ff6967e40..aa59062e69a4f2 100644 --- a/Include/warnings.h +++ b/Include/warnings.h @@ -56,6 +56,10 @@ PyErr_WarnExplicitFormat(PyObject *category, #define PyErr_Warn(category, msg) PyErr_WarnEx(category, msg, 1) #endif +#ifndef Py_LIMITED_API +PyAPI_FUNC(void) _PyErr_WarnUnawaitedCoroutine(PyObject *coro); +#endif + #ifdef __cplusplus } #endif diff --git a/Lib/test/test_coroutines.py b/Lib/test/test_coroutines.py index 8ada650a636b7b..9b060b86cf5459 100644 --- a/Lib/test/test_coroutines.py +++ b/Lib/test/test_coroutines.py @@ -6,6 +6,7 @@ import types import unittest import warnings +import re from test import support @@ -2086,6 +2087,79 @@ def nested(): finally: sys.set_coroutine_origin_tracking_depth(orig_depth) + def test_origin_tracking_warning(self): + async def corofn(): + pass + + a1_filename, a1_lineno = self.here() + def a1(): + return corofn() # comment in a1 + a1_lineno += 2 + + a2_filename, a2_lineno = self.here() + def a2(): + return a1() # comment in a2 + a2_lineno += 2 + + def check(depth, msg): + sys.set_coroutine_origin_tracking_depth(depth) + with warnings.catch_warnings(record=True) as wlist: + a2() + support.gc_collect() + # This might be fragile if other warnings somehow get triggered + # inside our 'with' block... let's worry about that if/when it + # happens. + self.assertTrue(len(wlist) == 1) + self.assertIs(wlist[0].category, RuntimeWarning) + self.assertEqual(msg, str(wlist[0].message)) + + orig_depth = sys.set_coroutine_origin_tracking_depth(0) + try: + msg = check(0, f"coroutine '{corofn.__qualname__}' was never awaited") + check(1, "".join([ + f"coroutine '{corofn.__qualname__}' was never awaited\n", + "Coroutine created at (most recent call last)\n", + f' File "{a1_filename}", line {a1_lineno}, in a1\n', + f' return corofn() # comment in a1', + ])) + check(2, "".join([ + f"coroutine '{corofn.__qualname__}' was never awaited\n", + "Coroutine created at (most recent call last)\n", + f' File "{a2_filename}", line {a2_lineno}, in a2\n', + f' return a1() # comment in a2\n', + f' File "{a1_filename}", line {a1_lineno}, in a1\n', + f' return corofn() # comment in a1', + ])) + + finally: + sys.set_coroutine_origin_tracking_depth(orig_depth) + + def test_unawaited_warning_when_module_broken(self): + # Make sure we don't blow up too bad if + # warnings._warn_unawaited_coroutine is broken somehow (e.g. because + # of shutdown problems) + async def corofn(): + pass + + orig_wuc = warnings._warn_unawaited_coroutine + try: + warnings._warn_unawaited_coroutine = lambda coro: 1/0 + with support.captured_stderr() as stream: + corofn() + support.gc_collect() + self.assertIn("Exception ignored in", stream.getvalue()) + self.assertIn("ZeroDivisionError", stream.getvalue()) + self.assertIn("was never awaited", stream.getvalue()) + + del warnings._warn_unawaited_coroutine + with support.captured_stderr() as stream: + corofn() + support.gc_collect() + self.assertIn("was never awaited", stream.getvalue()) + + finally: + warnings._warn_unawaited_coroutine = orig_wuc + @support.cpython_only class CAPITest(unittest.TestCase): diff --git a/Lib/warnings.py b/Lib/warnings.py index 76ad4dac018496..5db6641aa0c38b 100644 --- a/Lib/warnings.py +++ b/Lib/warnings.py @@ -488,6 +488,23 @@ def __exit__(self, *exc_info): self._module._showwarnmsg_impl = self._showwarnmsg_impl +# Private utility function called by _PyErr_WarnUnawaitedCoroutine +def _warn_unawaited_coroutine(coro): + msg_lines = [ + f"coroutine '{coro.__qualname__}' was never awaited\n" + ] + if coro.cr_origin is not None: + import linecache, traceback + def extract(): + for filename, lineno, funcname in reversed(coro.cr_origin): + line = linecache.getline(filename, lineno) + yield (filename, lineno, funcname, line) + msg_lines.append("Coroutine created at (most recent call last)\n") + msg_lines += traceback.format_list(list(extract())) + msg = "".join(msg_lines).rstrip("\n") + warn(msg, category=RuntimeWarning, stacklevel=2, source=coro) + + # filters contains a sequence of filter 5-tuples # The components of the 5-tuple are: # - an action: error, ignore, always, default, module, or once diff --git a/Objects/genobject.c b/Objects/genobject.c index 0a5dc7f50da1b6..1d3d3c90fe4aa0 100644 --- a/Objects/genobject.c +++ b/Objects/genobject.c @@ -76,11 +76,8 @@ _PyGen_Finalize(PyObject *self) if (gen->gi_code != NULL && ((PyCodeObject *)gen->gi_code)->co_flags & CO_COROUTINE && gen->gi_frame->f_lasti == -1) { - if (!error_value) { - PyErr_WarnFormat(PyExc_RuntimeWarning, 1, - "coroutine '%.50S' was never awaited", - gen->gi_qualname); - } + if (!error_value) + _PyErr_WarnUnawaitedCoroutine((PyObject *)gen); } else { res = gen_close(gen, NULL); diff --git a/Python/_warnings.c b/Python/_warnings.c index c286364dda03b2..87775419ac8f33 100644 --- a/Python/_warnings.c +++ b/Python/_warnings.c @@ -1153,6 +1153,52 @@ PyErr_WarnExplicitFormat(PyObject *category, return ret; } +void +_PyErr_WarnUnawaitedCoroutine(PyObject *coro) +{ + _Py_IDENTIFIER(_warn_unawaited_coroutine); + PyObject *fn = NULL, *res = NULL; + int warned = 0; + + /* First, we attempt to funnel the warning through + warnings._warn_unawaited_coroutine. + + This could raise an exception, due to: + - a bug + - some kind of shutdown-related brokenness + - succeeding, but with an "error" warning filter installed, so the + warning is converted into a RuntimeWarning exception + + In the first two cases, we want to print the error (so we know what it + is!), and then print a warning directly as a fallback. In the last + case, we want to print the error (since it's the warning!), but *not* + do a fallback. And after we print the error we can't check for what + type of error it was (because PyErr_WriteUnraisable clears it), so we + need a flag to keep track. + + Since this is called from __del__ context, it's careful to never raise + an exception. + */ + fn = get_warnings_attr(&PyId__warn_unawaited_coroutine, 1); + if (fn) { + res = PyObject_CallFunctionObjArgs(fn, coro, NULL); + if (res || PyErr_ExceptionMatches(PyExc_RuntimeWarning)) + warned = 1; + } + Py_XDECREF(fn); + Py_XDECREF(res); + + if (PyErr_Occurred()) + PyErr_WriteUnraisable(coro); + if (!warned) { + PyErr_WarnFormat(PyExc_RuntimeWarning, 1, + "coroutine '%.50S' was never awaited", + ((PyCoroObject *)coro)->cr_qualname); + /* Maybe *that* got converted into an exception */ + if (PyErr_Occurred()) + PyErr_WriteUnraisable(coro); + } +} PyDoc_STRVAR(warn_explicit_doc, "Low-level inferface to warnings functionality."); From 5d8f591823c3c1ecc1683d299c447eeb691780cd Mon Sep 17 00:00:00 2001 From: "Nathaniel J. Smith" Date: Sat, 20 Jan 2018 00:43:31 -0800 Subject: [PATCH 3/9] Stop using set_coroutine_wrapper in asyncio debug mode --- Lib/asyncio/base_events.py | 47 +++++++++------------------- Lib/asyncio/coroutines.py | 10 ------ Lib/test/test_asyncio/test_pep492.py | 39 +++++++---------------- 3 files changed, 27 insertions(+), 69 deletions(-) diff --git a/Lib/asyncio/base_events.py b/Lib/asyncio/base_events.py index 00c84a835c8bd1..6a542f389b5cb6 100644 --- a/Lib/asyncio/base_events.py +++ b/Lib/asyncio/base_events.py @@ -40,6 +40,7 @@ from . import sslproto from . import tasks from .log import logger +from .constants import DEBUG_STACK_DEPTH __all__ = 'BaseEventLoop', @@ -224,7 +225,8 @@ def __init__(self): self.slow_callback_duration = 0.1 self._current_handle = None self._task_factory = None - self._coroutine_wrapper_set = False + self._coroutine_origin_tracking_enabled = False + self._coroutine_origin_tracking_saved_depth = None if hasattr(sys, 'get_asyncgen_hooks'): # Python >= 3.6 @@ -382,7 +384,7 @@ def run_forever(self): if events._get_running_loop() is not None: raise RuntimeError( 'Cannot run the event loop while another loop is running') - self._set_coroutine_wrapper(self._debug) + self._set_coroutine_origin_tracking(self._debug) self._thread_id = threading.get_ident() if self._asyncgens is not None: old_agen_hooks = sys.get_asyncgen_hooks() @@ -398,7 +400,7 @@ def run_forever(self): self._stopping = False self._thread_id = None events._set_running_loop(None) - self._set_coroutine_wrapper(False) + self._set_coroutine_origin_tracking(False) if self._asyncgens is not None: sys.set_asyncgen_hooks(*old_agen_hooks) @@ -1531,39 +1533,20 @@ def _run_once(self): handle._run() handle = None # Needed to break cycles when an exception occurs. - def _set_coroutine_wrapper(self, enabled): - try: - set_wrapper = sys.set_coroutine_wrapper - get_wrapper = sys.get_coroutine_wrapper - except AttributeError: - return - - enabled = bool(enabled) - if self._coroutine_wrapper_set == enabled: + def _set_coroutine_origin_tracking(self, enabled): + if enabled == self._coroutine_origin_tracking_enabled: return - wrapper = coroutines.debug_wrapper - current_wrapper = get_wrapper() - if enabled: - if current_wrapper not in (None, wrapper): - warnings.warn( - f"loop.set_debug(True): cannot set debug coroutine " - f"wrapper; another wrapper is already set " - f"{current_wrapper!r}", - RuntimeWarning) - else: - set_wrapper(wrapper) - self._coroutine_wrapper_set = True + self._coroutine_origin_tracking_saved_depth = ( + sys.set_coroutine_origin_tracking_depth(DEBUG_STACK_DEPTH) + ) else: - if current_wrapper not in (None, wrapper): - warnings.warn( - f"loop.set_debug(False): cannot unset debug coroutine " - f"wrapper; another wrapper was set {current_wrapper!r}", - RuntimeWarning) - else: - set_wrapper(None) - self._coroutine_wrapper_set = False + sys.set_coroutine_origin_tracking_depth( + self._coroutine_origin_tracking_saved_depth + ) + + self._coroutine_origin_tracking_enabled = enabled def get_debug(self): return self._debug diff --git a/Lib/asyncio/coroutines.py b/Lib/asyncio/coroutines.py index 9c860a452b57c5..02f6285e05bb9a 100644 --- a/Lib/asyncio/coroutines.py +++ b/Lib/asyncio/coroutines.py @@ -32,14 +32,6 @@ def _is_debug_mode(): _DEBUG = _is_debug_mode() -def debug_wrapper(gen): - # This function is called from 'sys.set_coroutine_wrapper'. - # We only wrap here coroutines defined via 'async def' syntax. - # Generator-based coroutines are wrapped in @coroutine - # decorator. - return CoroWrapper(gen, None) - - class CoroWrapper: # Wrapper for coroutine object in _DEBUG mode. @@ -141,8 +133,6 @@ def coroutine(func): if inspect.iscoroutinefunction(func): # In Python 3.5 that's all we need to do for coroutines # defined with "async def". - # Wrapping in CoroWrapper will happen via - # 'sys.set_coroutine_wrapper' function. return func if inspect.isgeneratorfunction(func): diff --git a/Lib/test/test_asyncio/test_pep492.py b/Lib/test/test_asyncio/test_pep492.py index 289f7e59db719e..8ff5decfe6f108 100644 --- a/Lib/test/test_asyncio/test_pep492.py +++ b/Lib/test/test_asyncio/test_pep492.py @@ -1,5 +1,6 @@ """Tests support for new syntax introduced by PEP 492.""" +import sys import types import unittest @@ -148,35 +149,19 @@ async def foo(): data = self.loop.run_until_complete(foo()) self.assertEqual(data, 'spam') - @mock.patch('asyncio.coroutines.logger') - def test_async_def_wrapped(self, m_log): - async def foo(): - pass - async def start(): - foo_coro = foo() - self.assertRegex( - repr(foo_coro), - r'') - - with support.check_warnings((r'.*foo.*was never', - RuntimeWarning)): - foo_coro = None - support.gc_collect() - self.assertTrue(m_log.error.called) - message = m_log.error.call_args[0][0] - self.assertRegex(message, - r'CoroWrapper.*foo.*was never') - - self.loop.set_debug(True) - self.loop.run_until_complete(start()) + def test_debug_mode_manages_coroutine_origin_tracking(self): + def get_depth(): + depth = sys.set_coroutine_origin_tracking_depth(0) + sys.set_coroutine_origin_tracking_depth(depth) + return depth async def start(): - foo_coro = foo() - task = asyncio.ensure_future(foo_coro, loop=self.loop) - self.assertRegex(repr(task), r'Task.*foo.*running') + self.assertTrue(get_depth() > 0) + self.loop.set_debug(True) self.loop.run_until_complete(start()) + self.assertEqual(get_depth(), 0) def test_types_coroutine(self): def gen(): @@ -226,9 +211,9 @@ async def runner(): t.cancel() self.loop.set_debug(True) - with self.assertRaisesRegex( - RuntimeError, - r'Cannot await.*test_double_await.*\bafunc\b.*while.*\bsleep\b'): + with self.assertRaises( + RuntimeError, + msg='coroutine is being awaited already'): self.loop.run_until_complete(runner()) From 82d0c3ebc4755f19960d028238ac4d75b488fe59 Mon Sep 17 00:00:00 2001 From: "Nathaniel J. Smith" Date: Sat, 20 Jan 2018 00:43:54 -0800 Subject: [PATCH 4/9] In BaseEventLoop.set_debug, enable debugging in the correct thread This is fixing an old bug, that for some reason wasn't causing test suite failures until the previous commit. The issue was: Sometimes, for some reason, the tests are calling loop.set_debug() from a random thread, different from the one where the event loop is running. When set_debug() is called and the event loop is running, it attempts to immediately enable or disable debug stuff (previously registering/unregistering the coroutine wrapper, now toggling the coroutine origin tracking). However, it used to do this *in the thread where set_debug() was called*. Since the toggling uses a save/restore pattern, we'd end up saving the value in one thread and then restoring it in another, while never restoring it in the first thread, etc. Now we always enable/disable the debugging mode inside the event loop thread, which makes much more sense. And now the tests are passing again. --- Lib/asyncio/base_events.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/asyncio/base_events.py b/Lib/asyncio/base_events.py index 6a542f389b5cb6..b4da655877d568 100644 --- a/Lib/asyncio/base_events.py +++ b/Lib/asyncio/base_events.py @@ -1555,4 +1555,4 @@ def set_debug(self, enabled): self._debug = enabled if self.is_running(): - self._set_coroutine_wrapper(enabled) + self.call_soon_threadsafe(self._set_coroutine_origin_tracking, enabled) From 6c7f73a280af23778a21aae56cced970a0204ae1 Mon Sep 17 00:00:00 2001 From: "Nathaniel J. Smith" Date: Sat, 20 Jan 2018 00:50:36 -0800 Subject: [PATCH 5/9] Add NEWS blurb --- .../2018-01-20-00-50-33.bpo-32591.666kl6.rst | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2018-01-20-00-50-33.bpo-32591.666kl6.rst diff --git a/Misc/NEWS.d/next/Core and Builtins/2018-01-20-00-50-33.bpo-32591.666kl6.rst b/Misc/NEWS.d/next/Core and Builtins/2018-01-20-00-50-33.bpo-32591.666kl6.rst new file mode 100644 index 00000000000000..e3f3d59aca7440 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2018-01-20-00-50-33.bpo-32591.666kl6.rst @@ -0,0 +1,4 @@ +Added built-in support for tracking the origin of coroutine objects; see +sys.set_coroutine_origin_tracking_depth and CoroutineType.cr_origin. This +replaces the asyncio debug mode's use of coroutine wrapping for native +coroutine objects. From 7738cc4d85f6f96c7cffd70b62d5dfd8c25d125c Mon Sep 17 00:00:00 2001 From: "Nathaniel J. Smith" Date: Sat, 20 Jan 2018 00:52:01 -0800 Subject: [PATCH 6/9] Guess I should add myself to ACKS at some point --- Misc/ACKS | 1 + 1 file changed, 1 insertion(+) diff --git a/Misc/ACKS b/Misc/ACKS index 009b072d680aac..900604ca4c6a13 100644 --- a/Misc/ACKS +++ b/Misc/ACKS @@ -1486,6 +1486,7 @@ Christopher Smith Eric V. Smith Gregory P. Smith Mark Smith +Nathaniel J. Smith Roy Smith Ryan Smith-Roberts Rafal Smotrzyk From 2157af3032fe196f1088b37248641e7b09e6686b Mon Sep 17 00:00:00 2001 From: "Nathaniel J. Smith" Date: Sat, 20 Jan 2018 20:18:35 -0800 Subject: [PATCH 7/9] Address most of Yury's comments --- Doc/library/inspect.rst | 7 +++- Doc/library/sys.rst | 15 ++++++-- Doc/whatsnew/3.7.rst | 3 ++ Include/warnings.h | 2 +- Lib/asyncio/base_events.py | 11 +++--- Lib/asyncio/coroutines.py | 23 ------------ Lib/test/test_coroutines.py | 20 +++++----- Lib/warnings.py | 6 +++ Objects/genobject.c | 74 +++++++++++++++++++++---------------- Python/_warnings.c | 23 ++++++------ Python/ceval.c | 1 + Python/sysmodule.c | 9 +++++ 12 files changed, 108 insertions(+), 86 deletions(-) diff --git a/Doc/library/inspect.rst b/Doc/library/inspect.rst index 2e540f467049ba..147e802cac4be6 100644 --- a/Doc/library/inspect.rst +++ b/Doc/library/inspect.rst @@ -34,6 +34,9 @@ provided as convenient choices for the second argument to :func:`getmembers`. They also help you determine when you can expect to find the following special attributes: +.. this function name is too big to fit in the ascii-art table below +.. |coroutine-origin-link| replace:: :func:`sys.set_coroutine_origin_tracking_depth` + +-----------+-------------------+---------------------------+ | Type | Attribute | Description | +===========+===================+===========================+ @@ -216,8 +219,8 @@ attributes: | | cr_code | code | +-----------+-------------------+---------------------------+ | | cr_origin | where coroutine was | -| | | created, if coroutine | -| | | origin tracking is enabled| +| | | created, or ``None``. See | +| | | |coroutine-origin-link| | +-----------+-------------------+---------------------------+ | builtin | __doc__ | documentation string | +-----------+-------------------+---------------------------+ diff --git a/Doc/library/sys.rst b/Doc/library/sys.rst index 88e9fcf9e42e11..70e6eff9d590ee 100644 --- a/Doc/library/sys.rst +++ b/Doc/library/sys.rst @@ -686,6 +686,10 @@ always available. This function has been added on a provisional basis (see :pep:`411` for details.) Use it only for debugging purposes. + .. deprecated:: 3.7 + The coroutine wrapper functionality has been deprecated, and + will be removed in 3.8. See :issue:`32591` for details. + .. data:: hash_info @@ -1217,8 +1221,9 @@ always available. Allows enabling or disabling coroutine origin tracking. When enabled, the ``cr_origin`` attribute on coroutine objects will contain a list of (filename, line number, function name) tuples - describing the traceback where the coroutine object was created. - When disabled, ``cr_origin`` will be None. + describing the traceback where the coroutine object was created, + with the most recent call first. When disabled, ``cr_origin`` will + be None. To enable, pass a *depth* value greater than zero; this sets the number of frames whose information will be captured. To disable, @@ -1226,7 +1231,7 @@ always available. Returns the old value of *depth*. - This setting is thread-local. + This setting is thread-specific. .. versionadded:: 3.7 @@ -1273,6 +1278,10 @@ always available. This function has been added on a provisional basis (see :pep:`411` for details.) Use it only for debugging purposes. + .. deprecated:: 3.7 + The coroutine wrapper functionality has been deprecated, and + will be removed in 3.8. See :issue:`32591` for details. + .. function:: _enablelegacywindowsfsencoding() Changes the default filesystem encoding and errors mode to 'mbcs' and diff --git a/Doc/whatsnew/3.7.rst b/Doc/whatsnew/3.7.rst index 009df38d8ece19..54a3f14b0bb4f2 100644 --- a/Doc/whatsnew/3.7.rst +++ b/Doc/whatsnew/3.7.rst @@ -510,6 +510,9 @@ sys Added :attr:`sys.flags.dev_mode` flag for the new development mode. +Deprecated :func:`sys.set_coroutine_wrapper` and +:func:`sys.get_coroutine_wrapper`. + time ---- diff --git a/Include/warnings.h b/Include/warnings.h index aa59062e69a4f2..a675bb5dfcb9f5 100644 --- a/Include/warnings.h +++ b/Include/warnings.h @@ -57,7 +57,7 @@ PyErr_WarnExplicitFormat(PyObject *category, #endif #ifndef Py_LIMITED_API -PyAPI_FUNC(void) _PyErr_WarnUnawaitedCoroutine(PyObject *coro); +void _PyErr_WarnUnawaitedCoroutine(PyObject *coro); #endif #ifdef __cplusplus diff --git a/Lib/asyncio/base_events.py b/Lib/asyncio/base_events.py index b4da655877d568..1b7c29e54f64ad 100644 --- a/Lib/asyncio/base_events.py +++ b/Lib/asyncio/base_events.py @@ -34,13 +34,13 @@ except ImportError: # pragma: no cover ssl = None +from . import constants from . import coroutines from . import events from . import futures from . import sslproto from . import tasks from .log import logger -from .constants import DEBUG_STACK_DEPTH __all__ = 'BaseEventLoop', @@ -1534,17 +1534,16 @@ def _run_once(self): handle = None # Needed to break cycles when an exception occurs. def _set_coroutine_origin_tracking(self, enabled): - if enabled == self._coroutine_origin_tracking_enabled: + if bool(enabled) == bool(self._coroutine_origin_tracking_enabled): return if enabled: self._coroutine_origin_tracking_saved_depth = ( - sys.set_coroutine_origin_tracking_depth(DEBUG_STACK_DEPTH) - ) + sys.set_coroutine_origin_tracking_depth( + constants.DEBUG_STACK_DEPTH)) else: sys.set_coroutine_origin_tracking_depth( - self._coroutine_origin_tracking_saved_depth - ) + self._coroutine_origin_tracking_saved_depth) self._coroutine_origin_tracking_enabled = enabled diff --git a/Lib/asyncio/coroutines.py b/Lib/asyncio/coroutines.py index 02f6285e05bb9a..5a29100321fae9 100644 --- a/Lib/asyncio/coroutines.py +++ b/Lib/asyncio/coroutines.py @@ -79,39 +79,16 @@ def gi_code(self): return self.gen.gi_code def __await__(self): - cr_await = getattr(self.gen, 'cr_await', None) - if cr_await is not None: - raise RuntimeError( - f"Cannot await on coroutine {self.gen!r} while it's " - f"awaiting for {cr_await!r}") return self @property def gi_yieldfrom(self): return self.gen.gi_yieldfrom - @property - def cr_await(self): - return self.gen.cr_await - - @property - def cr_running(self): - return self.gen.cr_running - - @property - def cr_code(self): - return self.gen.cr_code - - @property - def cr_frame(self): - return self.gen.cr_frame - def __del__(self): # Be careful accessing self.gen.frame -- self.gen might not exist. gen = getattr(self, 'gen', None) frame = getattr(gen, 'gi_frame', None) - if frame is None: - frame = getattr(gen, 'cr_frame', None) if frame is not None and frame.f_lasti == -1: msg = f'{self!r} was never yielded from' tb = getattr(self, '_source_traceback', ()) diff --git a/Lib/test/test_coroutines.py b/Lib/test/test_coroutines.py index 9b060b86cf5459..323f8f13ce5075 100644 --- a/Lib/test/test_coroutines.py +++ b/Lib/test/test_coroutines.py @@ -1,12 +1,12 @@ -from contextlib import contextmanager, closing +import contextlib import copy import inspect import pickle +import re import sys import types import unittest import warnings -import re from test import support @@ -59,7 +59,7 @@ def run_async__await__(coro): return buffer, result -@contextmanager +@contextlib.contextmanager def silence_coro_gc(): with warnings.catch_warnings(): warnings.simplefilter("ignore") @@ -1975,9 +1975,11 @@ def wrap(gen): wrapped = gen return gen - self.assertIsNone(sys.get_coroutine_wrapper()) + with self.assertWarns(DeprecationWarning): + self.assertIsNone(sys.get_coroutine_wrapper()) - sys.set_coroutine_wrapper(wrap) + with self.assertWarns(DeprecationWarning): + sys.set_coroutine_wrapper(wrap) self.assertIs(sys.get_coroutine_wrapper(), wrap) try: f = foo() @@ -2053,13 +2055,13 @@ def test_origin_tracking(self): async def corofn(): pass - with closing(corofn()) as coro: + with contextlib.closing(corofn()) as coro: self.assertIsNone(coro.cr_origin) self.assertEqual(sys.set_coroutine_origin_tracking_depth(1), 0) fname, lineno = self.here() - with closing(corofn()) as coro: + with contextlib.closing(corofn()) as coro: self.assertEqual(coro.cr_origin, [(fname, lineno + 1, "test_origin_tracking")]) @@ -2068,14 +2070,14 @@ def nested(): return (self.here(), corofn()) fname, lineno = self.here() ((nested_fname, nested_lineno), coro) = nested() - with closing(coro): + with contextlib.closing(coro): self.assertEqual(coro.cr_origin, [(nested_fname, nested_lineno, "nested"), (fname, lineno + 1, "test_origin_tracking")]) # Check we handle running out of frames correctly sys.set_coroutine_origin_tracking_depth(1000) - with closing(corofn()) as coro: + with contextlib.closing(corofn()) as coro: self.assertTrue(2 < len(coro.cr_origin) < 1000) # We can't set depth negative diff --git a/Lib/warnings.py b/Lib/warnings.py index 5db6641aa0c38b..81f98647786d2c 100644 --- a/Lib/warnings.py +++ b/Lib/warnings.py @@ -502,6 +502,12 @@ def extract(): msg_lines.append("Coroutine created at (most recent call last)\n") msg_lines += traceback.format_list(list(extract())) msg = "".join(msg_lines).rstrip("\n") + # Passing source= here means that if the user happens to have tracemalloc + # enabled and tracking where the coroutine was created, the warning will + # contain that traceback. This does mean that if they have *both* + # coroutine origin tracking *and* tracemalloc enabled, they'll get two + # partially-redundant tracebacks. If we wanted to be clever we could + # probably detect this case and avoid it, but for now we don't bother. warn(msg, category=RuntimeWarning, stacklevel=2, source=coro) diff --git a/Objects/genobject.c b/Objects/genobject.c index 1d3d3c90fe4aa0..5f10236951c2bc 100644 --- a/Objects/genobject.c +++ b/Objects/genobject.c @@ -32,8 +32,9 @@ gen_traverse(PyGenObject *gen, visitproc visit, void *arg) Py_VISIT(gen->gi_code); Py_VISIT(gen->gi_name); Py_VISIT(gen->gi_qualname); - if (((PyCodeObject *)gen->gi_code)->co_flags & CO_COROUTINE) + if (((PyCodeObject *)gen->gi_code)->co_flags & CO_COROUTINE) { Py_VISIT(((PyCoroObject *)gen)->cr_origin); + } return exc_state_traverse(&gen->gi_exc_state, visit, arg); } @@ -76,8 +77,9 @@ _PyGen_Finalize(PyObject *self) if (gen->gi_code != NULL && ((PyCodeObject *)gen->gi_code)->co_flags & CO_COROUTINE && gen->gi_frame->f_lasti == -1) { - if (!error_value) + if (!error_value) { _PyErr_WarnUnawaitedCoroutine((PyObject *)gen); + } } else { res = gen_close(gen, NULL); @@ -136,8 +138,9 @@ gen_dealloc(PyGenObject *gen) gen->gi_frame->f_gen = NULL; Py_CLEAR(gen->gi_frame); } - if (((PyCodeObject *)gen->gi_code)->co_flags & CO_COROUTINE) + if (((PyCodeObject *)gen->gi_code)->co_flags & CO_COROUTINE) { Py_CLEAR(((PyCoroObject *)gen)->cr_origin); + } Py_CLEAR(gen->gi_code); Py_CLEAR(gen->gi_name); Py_CLEAR(gen->gi_qualname); @@ -1160,47 +1163,56 @@ PyTypeObject _PyCoroWrapper_Type = { 0, /* tp_free */ }; +static PyObject * +compute_cr_origin(int origin_depth) +{ + PyObject *cr_origin = PyList_New(origin_depth); + PyFrameObject *frame = PyEval_GetFrame(); + int i = 0; + for (; frame && i < origin_depth; ++i) { + PyObject *frameinfo = Py_BuildValue( + "OiO", + frame->f_code->co_filename, + PyFrame_GetLineNumber(frame), + frame->f_code->co_name); + if (!frameinfo) { + Py_DECREF(cr_origin); + return NULL; + } + PyList_SET_ITEM(cr_origin, i, frameinfo); + frame = frame->f_back; + } + /* Truncate the list if necessary */ + if (i < origin_depth) { + if (PyList_SetSlice(cr_origin, i, origin_depth, NULL) < 0) { + Py_DECREF(cr_origin); + return NULL; + } + } + + return cr_origin; +} + PyObject * PyCoro_New(PyFrameObject *f, PyObject *name, PyObject *qualname) { PyObject *coro = gen_new_with_qualname(&PyCoro_Type, f, name, qualname); - if (!coro) + if (!coro) { return NULL; + } PyThreadState *tstate = PyThreadState_GET(); - int depth = tstate->coroutine_origin_tracking_depth; + int origin_depth = tstate->coroutine_origin_tracking_depth; - if (depth == 0) { + if (origin_depth == 0) { ((PyCoroObject *)coro)->cr_origin = NULL; } else { - PyObject *origin = PyList_New(depth); - /* Immediately pass ownership to coro, so on error paths we don't have - to worry about it separately. */ - ((PyCoroObject *)coro)->cr_origin = origin; - PyFrameObject *frame = PyEval_GetFrame(); - int i = 0; - for (; i < depth; ++i) { - if (!frame) - break; - - PyObject *frameinfo = Py_BuildValue( - "OiO", - frame->f_code->co_filename, - PyFrame_GetLineNumber(frame), - frame->f_code->co_name); - if (!frameinfo) { - Py_DECREF(coro); - return NULL; - } - PyList_SET_ITEM(origin, i, frameinfo); - - frame = frame->f_back; - } - /* Truncate the list if necessary */ - if (PyList_SetSlice(origin, i, depth, NULL) < 0) { + PyObject *cr_origin = compute_cr_origin(origin_depth); + if (!cr_origin) { Py_DECREF(coro); return NULL; } + ((PyCoroObject *)coro)->cr_origin = cr_origin; } return coro; diff --git a/Python/_warnings.c b/Python/_warnings.c index 87775419ac8f33..c3417cccb179d2 100644 --- a/Python/_warnings.c +++ b/Python/_warnings.c @@ -1156,10 +1156,6 @@ PyErr_WarnExplicitFormat(PyObject *category, void _PyErr_WarnUnawaitedCoroutine(PyObject *coro) { - _Py_IDENTIFIER(_warn_unawaited_coroutine); - PyObject *fn = NULL, *res = NULL; - int warned = 0; - /* First, we attempt to funnel the warning through warnings._warn_unawaited_coroutine. @@ -1179,24 +1175,29 @@ _PyErr_WarnUnawaitedCoroutine(PyObject *coro) Since this is called from __del__ context, it's careful to never raise an exception. */ - fn = get_warnings_attr(&PyId__warn_unawaited_coroutine, 1); + _Py_IDENTIFIER(_warn_unawaited_coroutine); + int warned = 0; + PyObject *fn = get_warnings_attr(&PyId__warn_unawaited_coroutine, 1); if (fn) { - res = PyObject_CallFunctionObjArgs(fn, coro, NULL); - if (res || PyErr_ExceptionMatches(PyExc_RuntimeWarning)) + PyObject *res = PyObject_CallFunctionObjArgs(fn, coro, NULL); + Py_DECREF(fn); + if (res || PyErr_ExceptionMatches(PyExc_RuntimeWarning)) { warned = 1; + } + Py_XDECREF(res); } - Py_XDECREF(fn); - Py_XDECREF(res); - if (PyErr_Occurred()) + if (PyErr_Occurred()) { PyErr_WriteUnraisable(coro); + } if (!warned) { PyErr_WarnFormat(PyExc_RuntimeWarning, 1, "coroutine '%.50S' was never awaited", ((PyCoroObject *)coro)->cr_qualname); /* Maybe *that* got converted into an exception */ - if (PyErr_Occurred()) + if (PyErr_Occurred()) { PyErr_WriteUnraisable(coro); + } } } diff --git a/Python/ceval.c b/Python/ceval.c index aecba34b8dbce8..96cc7464eb5623 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -4390,6 +4390,7 @@ PyEval_SetTrace(Py_tracefunc func, PyObject *arg) int _PyEval_SetCoroutineOriginTrackingDepth(int new_depth) { + assert(new_depth >= 0); PyThreadState *tstate = PyThreadState_GET(); int old_depth = tstate->coroutine_origin_tracking_depth; tstate->coroutine_origin_tracking_depth = new_depth; diff --git a/Python/sysmodule.c b/Python/sysmodule.c index 6c868b8b5e880c..14cd6c4d87ab32 100644 --- a/Python/sysmodule.c +++ b/Python/sysmodule.c @@ -743,6 +743,11 @@ sys_set_coroutine_origin_tracking_depth_impl(PyObject *module, int depth) static PyObject * sys_set_coroutine_wrapper(PyObject *self, PyObject *wrapper) { + if (PyErr_WarnEx(PyExc_DeprecationWarning, + "set_coroutine_wrapper is deprecated", 1) < 0) { + return NULL; + } + if (wrapper != Py_None) { if (!PyCallable_Check(wrapper)) { PyErr_Format(PyExc_TypeError, @@ -767,6 +772,10 @@ Set a wrapper for coroutine objects." static PyObject * sys_get_coroutine_wrapper(PyObject *self, PyObject *args) { + if (PyErr_WarnEx(PyExc_DeprecationWarning, + "get_coroutine_wrapper is deprecated", 1) < 0) { + return NULL; + } PyObject *wrapper = _PyEval_GetCoroutineWrapper(); if (wrapper == NULL) { wrapper = Py_None; From b0e52ecbd8ca4a016c410a44369adcd0f7a5c263 Mon Sep 17 00:00:00 2001 From: "Nathaniel J. Smith" Date: Sat, 20 Jan 2018 21:34:46 -0800 Subject: [PATCH 8/9] Switch coroutine origin tracking to a get/set API --- Doc/library/sys.rst | 14 ++++++++++-- Include/ceval.h | 3 ++- Lib/asyncio/base_events.py | 5 +++-- Lib/test/test_asyncio/test_pep492.py | 11 +++------- Lib/test/test_coroutines.py | 16 +++++++++----- Python/ceval.c | 11 +++++++--- Python/clinic/sysmodule.c.h | 33 +++++++++++++++++++++++----- Python/sysmodule.c | 27 ++++++++++++++++++----- 8 files changed, 88 insertions(+), 32 deletions(-) diff --git a/Doc/library/sys.rst b/Doc/library/sys.rst index 70e6eff9d590ee..be215dde3e97af 100644 --- a/Doc/library/sys.rst +++ b/Doc/library/sys.rst @@ -675,6 +675,18 @@ always available. for details.) +.. function:: get_coroutine_origin_tracking_depth() + + Get the current coroutine origin tracking depth, as set by + func:`set_coroutine_origin_tracking_depth`. + + .. versionadded:: 3.7 + + .. note:: + This function has been added on a provisional basis (see :pep:`411` + for details.) Use it only for debugging purposes. + + .. function:: get_coroutine_wrapper() Returns ``None``, or a wrapper set by :func:`set_coroutine_wrapper`. @@ -1229,8 +1241,6 @@ always available. number of frames whose information will be captured. To disable, pass set *depth* to zero. - Returns the old value of *depth*. - This setting is thread-specific. .. versionadded:: 3.7 diff --git a/Include/ceval.h b/Include/ceval.h index 4066132b931d9e..bce8a0beed85e1 100644 --- a/Include/ceval.h +++ b/Include/ceval.h @@ -31,7 +31,8 @@ PyAPI_FUNC(PyObject *) PyEval_CallMethod(PyObject *obj, #ifndef Py_LIMITED_API PyAPI_FUNC(void) PyEval_SetProfile(Py_tracefunc, PyObject *); PyAPI_FUNC(void) PyEval_SetTrace(Py_tracefunc, PyObject *); -PyAPI_FUNC(int) _PyEval_SetCoroutineOriginTrackingDepth(int new_depth); +PyAPI_FUNC(void) _PyEval_SetCoroutineOriginTrackingDepth(int new_depth); +PyAPI_FUNC(int) _PyEval_GetCoroutineOriginTrackingDepth(void); PyAPI_FUNC(void) _PyEval_SetCoroutineWrapper(PyObject *); PyAPI_FUNC(PyObject *) _PyEval_GetCoroutineWrapper(void); PyAPI_FUNC(void) _PyEval_SetAsyncGenFirstiter(PyObject *); diff --git a/Lib/asyncio/base_events.py b/Lib/asyncio/base_events.py index 1b7c29e54f64ad..a10e706d504211 100644 --- a/Lib/asyncio/base_events.py +++ b/Lib/asyncio/base_events.py @@ -1539,8 +1539,9 @@ def _set_coroutine_origin_tracking(self, enabled): if enabled: self._coroutine_origin_tracking_saved_depth = ( - sys.set_coroutine_origin_tracking_depth( - constants.DEBUG_STACK_DEPTH)) + sys.get_coroutine_origin_tracking_depth()) + sys.set_coroutine_origin_tracking_depth( + constants.DEBUG_STACK_DEPTH) else: sys.set_coroutine_origin_tracking_depth( self._coroutine_origin_tracking_saved_depth) diff --git a/Lib/test/test_asyncio/test_pep492.py b/Lib/test/test_asyncio/test_pep492.py index 8ff5decfe6f108..f2d588f54445a2 100644 --- a/Lib/test/test_asyncio/test_pep492.py +++ b/Lib/test/test_asyncio/test_pep492.py @@ -150,18 +150,13 @@ async def foo(): self.assertEqual(data, 'spam') def test_debug_mode_manages_coroutine_origin_tracking(self): - def get_depth(): - depth = sys.set_coroutine_origin_tracking_depth(0) - sys.set_coroutine_origin_tracking_depth(depth) - return depth - async def start(): - self.assertTrue(get_depth() > 0) + self.assertTrue(sys.get_coroutine_origin_tracking_depth() > 0) + self.assertEqual(sys.get_coroutine_origin_tracking_depth(), 0) self.loop.set_debug(True) self.loop.run_until_complete(start()) - - self.assertEqual(get_depth(), 0) + self.assertEqual(sys.get_coroutine_origin_tracking_depth(), 0) def test_types_coroutine(self): def gen(): diff --git a/Lib/test/test_coroutines.py b/Lib/test/test_coroutines.py index 323f8f13ce5075..35b40d632b001b 100644 --- a/Lib/test/test_coroutines.py +++ b/Lib/test/test_coroutines.py @@ -2050,22 +2050,28 @@ def here(self): return (info.filename, info.lineno) def test_origin_tracking(self): - orig_depth = sys.set_coroutine_origin_tracking_depth(0) + orig_depth = sys.get_coroutine_origin_tracking_depth() try: async def corofn(): pass + sys.set_coroutine_origin_tracking_depth(0) + self.assertEqual(sys.get_coroutine_origin_tracking_depth(), 0) + with contextlib.closing(corofn()) as coro: self.assertIsNone(coro.cr_origin) - self.assertEqual(sys.set_coroutine_origin_tracking_depth(1), 0) + sys.set_coroutine_origin_tracking_depth(1) + self.assertEqual(sys.get_coroutine_origin_tracking_depth(), 1) fname, lineno = self.here() with contextlib.closing(corofn()) as coro: self.assertEqual(coro.cr_origin, [(fname, lineno + 1, "test_origin_tracking")]) - self.assertEqual(sys.set_coroutine_origin_tracking_depth(2), 1) + sys.set_coroutine_origin_tracking_depth(2) + self.assertEqual(sys.get_coroutine_origin_tracking_depth(), 2) + def nested(): return (self.here(), corofn()) fname, lineno = self.here() @@ -2084,7 +2090,7 @@ def nested(): with self.assertRaises(ValueError): sys.set_coroutine_origin_tracking_depth(-1) # And trying leaves it unchanged - self.assertEqual(sys.set_coroutine_origin_tracking_depth(0), 1000) + self.assertEqual(sys.get_coroutine_origin_tracking_depth(), 1000) finally: sys.set_coroutine_origin_tracking_depth(orig_depth) @@ -2115,7 +2121,7 @@ def check(depth, msg): self.assertIs(wlist[0].category, RuntimeWarning) self.assertEqual(msg, str(wlist[0].message)) - orig_depth = sys.set_coroutine_origin_tracking_depth(0) + orig_depth = sys.get_coroutine_origin_tracking_depth() try: msg = check(0, f"coroutine '{corofn.__qualname__}' was never awaited") check(1, "".join([ diff --git a/Python/ceval.c b/Python/ceval.c index 96cc7464eb5623..2b7c0c80242d86 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -4387,14 +4387,19 @@ PyEval_SetTrace(Py_tracefunc func, PyObject *arg) || (tstate->c_profilefunc != NULL)); } -int +void _PyEval_SetCoroutineOriginTrackingDepth(int new_depth) { assert(new_depth >= 0); PyThreadState *tstate = PyThreadState_GET(); - int old_depth = tstate->coroutine_origin_tracking_depth; tstate->coroutine_origin_tracking_depth = new_depth; - return old_depth; +} + +int +_PyEval_GetCoroutineOriginTrackingDepth(void) +{ + PyThreadState *tstate = PyThreadState_GET(); + return tstate->coroutine_origin_tracking_depth; } void diff --git a/Python/clinic/sysmodule.c.h b/Python/clinic/sysmodule.c.h index b68b699d05810b..3e1480513f6c2a 100644 --- a/Python/clinic/sysmodule.c.h +++ b/Python/clinic/sysmodule.c.h @@ -10,12 +10,12 @@ PyDoc_STRVAR(sys_set_coroutine_origin_tracking_depth__doc__, "\n" "Coroutine objects will track \'depth\' frames of traceback information about\n" "where they came from, available in their cr_origin attribute. Set depth of 0\n" -"to disable. Returns old value."); +"to disable."); #define SYS_SET_COROUTINE_ORIGIN_TRACKING_DEPTH_METHODDEF \ {"set_coroutine_origin_tracking_depth", (PyCFunction)sys_set_coroutine_origin_tracking_depth, METH_FASTCALL|METH_KEYWORDS, sys_set_coroutine_origin_tracking_depth__doc__}, -static int +static PyObject * sys_set_coroutine_origin_tracking_depth_impl(PyObject *module, int depth); static PyObject * @@ -25,13 +25,36 @@ sys_set_coroutine_origin_tracking_depth(PyObject *module, PyObject *const *args, static const char * const _keywords[] = {"depth", NULL}; static _PyArg_Parser _parser = {"i:set_coroutine_origin_tracking_depth", _keywords, 0}; int depth; - int _return_value; if (!_PyArg_ParseStackAndKeywords(args, nargs, kwnames, &_parser, &depth)) { goto exit; } - _return_value = sys_set_coroutine_origin_tracking_depth_impl(module, depth); + return_value = sys_set_coroutine_origin_tracking_depth_impl(module, depth); + +exit: + return return_value; +} + +PyDoc_STRVAR(sys_get_coroutine_origin_tracking_depth__doc__, +"get_coroutine_origin_tracking_depth($module, /)\n" +"--\n" +"\n" +"Check status of origin tracking for coroutine objects in this thread."); + +#define SYS_GET_COROUTINE_ORIGIN_TRACKING_DEPTH_METHODDEF \ + {"get_coroutine_origin_tracking_depth", (PyCFunction)sys_get_coroutine_origin_tracking_depth, METH_NOARGS, sys_get_coroutine_origin_tracking_depth__doc__}, + +static int +sys_get_coroutine_origin_tracking_depth_impl(PyObject *module); + +static PyObject * +sys_get_coroutine_origin_tracking_depth(PyObject *module, PyObject *Py_UNUSED(ignored)) +{ + PyObject *return_value = NULL; + int _return_value; + + _return_value = sys_get_coroutine_origin_tracking_depth_impl(module); if ((_return_value == -1) && PyErr_Occurred()) { goto exit; } @@ -40,4 +63,4 @@ sys_set_coroutine_origin_tracking_depth(PyObject *module, PyObject *const *args, exit: return return_value; } -/*[clinic end generated code: output=9448b0765e4d5bf0 input=a9049054013a1b77]*/ +/*[clinic end generated code: output=4a3ac42b97d710ff input=a9049054013a1b77]*/ diff --git a/Python/sysmodule.c b/Python/sysmodule.c index 14cd6c4d87ab32..873657f4a4f119 100644 --- a/Python/sysmodule.c +++ b/Python/sysmodule.c @@ -718,7 +718,7 @@ sys_setrecursionlimit(PyObject *self, PyObject *args) } /*[clinic input] -sys.set_coroutine_origin_tracking_depth -> int +sys.set_coroutine_origin_tracking_depth depth: int @@ -726,18 +726,32 @@ Enable or disable origin tracking for coroutine objects in this thread. Coroutine objects will track 'depth' frames of traceback information about where they came from, available in their cr_origin attribute. Set depth of 0 -to disable. Returns old value. +to disable. [clinic start generated code]*/ -static int +static PyObject * sys_set_coroutine_origin_tracking_depth_impl(PyObject *module, int depth) -/*[clinic end generated code: output=1d421106d83a2fce input=f0a48609fc463a5c]*/ +/*[clinic end generated code: output=0a2123c1cc6759c5 input=9083112cccc1bdcb]*/ { if (depth < 0) { PyErr_SetString(PyExc_ValueError, "depth must be >= 0"); - return -1; + return NULL; } - return _PyEval_SetCoroutineOriginTrackingDepth(depth); + _PyEval_SetCoroutineOriginTrackingDepth(depth); + Py_RETURN_NONE; +} + +/*[clinic input] +sys.get_coroutine_origin_tracking_depth -> int + +Check status of origin tracking for coroutine objects in this thread. +[clinic start generated code]*/ + +static int +sys_get_coroutine_origin_tracking_depth_impl(PyObject *module) +/*[clinic end generated code: output=3699f7be95a3afb8 input=335266a71205b61a]*/ +{ + return _PyEval_GetCoroutineOriginTrackingDepth(); } static PyObject * @@ -1552,6 +1566,7 @@ static PyMethodDef sys_methods[] = { {"_debugmallocstats", sys_debugmallocstats, METH_NOARGS, debugmallocstats_doc}, SYS_SET_COROUTINE_ORIGIN_TRACKING_DEPTH_METHODDEF + SYS_GET_COROUTINE_ORIGIN_TRACKING_DEPTH_METHODDEF {"set_coroutine_wrapper", sys_set_coroutine_wrapper, METH_O, set_coroutine_wrapper_doc}, {"get_coroutine_wrapper", sys_get_coroutine_wrapper, METH_NOARGS, From c0feb3b5a731da53cbf767df253bef9fcaf3d67b Mon Sep 17 00:00:00 2001 From: "Nathaniel J. Smith" Date: Sat, 20 Jan 2018 21:39:47 -0800 Subject: [PATCH 9/9] Make cr_origin a tuple --- Doc/library/sys.rst | 2 +- Lib/test/test_coroutines.py | 6 +++--- Objects/genobject.c | 27 +++++++++++++-------------- 3 files changed, 17 insertions(+), 18 deletions(-) diff --git a/Doc/library/sys.rst b/Doc/library/sys.rst index be215dde3e97af..54281a3f9a0c15 100644 --- a/Doc/library/sys.rst +++ b/Doc/library/sys.rst @@ -1232,7 +1232,7 @@ always available. Allows enabling or disabling coroutine origin tracking. When enabled, the ``cr_origin`` attribute on coroutine objects will - contain a list of (filename, line number, function name) tuples + contain a tuple of (filename, line number, function name) tuples describing the traceback where the coroutine object was created, with the most recent call first. When disabled, ``cr_origin`` will be None. diff --git a/Lib/test/test_coroutines.py b/Lib/test/test_coroutines.py index 35b40d632b001b..8a531b888924e3 100644 --- a/Lib/test/test_coroutines.py +++ b/Lib/test/test_coroutines.py @@ -2067,7 +2067,7 @@ async def corofn(): fname, lineno = self.here() with contextlib.closing(corofn()) as coro: self.assertEqual(coro.cr_origin, - [(fname, lineno + 1, "test_origin_tracking")]) + ((fname, lineno + 1, "test_origin_tracking"),)) sys.set_coroutine_origin_tracking_depth(2) self.assertEqual(sys.get_coroutine_origin_tracking_depth(), 2) @@ -2078,8 +2078,8 @@ def nested(): ((nested_fname, nested_lineno), coro) = nested() with contextlib.closing(coro): self.assertEqual(coro.cr_origin, - [(nested_fname, nested_lineno, "nested"), - (fname, lineno + 1, "test_origin_tracking")]) + ((nested_fname, nested_lineno, "nested"), + (fname, lineno + 1, "test_origin_tracking"))) # Check we handle running out of frames correctly sys.set_coroutine_origin_tracking_depth(1000) diff --git a/Objects/genobject.c b/Objects/genobject.c index 5f10236951c2bc..0a34c1f6a97fe2 100644 --- a/Objects/genobject.c +++ b/Objects/genobject.c @@ -32,9 +32,8 @@ gen_traverse(PyGenObject *gen, visitproc visit, void *arg) Py_VISIT(gen->gi_code); Py_VISIT(gen->gi_name); Py_VISIT(gen->gi_qualname); - if (((PyCodeObject *)gen->gi_code)->co_flags & CO_COROUTINE) { - Py_VISIT(((PyCoroObject *)gen)->cr_origin); - } + /* No need to visit cr_origin, because it's just tuples/str/int, so can't + participate in a reference cycle. */ return exc_state_traverse(&gen->gi_exc_state, visit, arg); } @@ -1166,10 +1165,17 @@ PyTypeObject _PyCoroWrapper_Type = { static PyObject * compute_cr_origin(int origin_depth) { - PyObject *cr_origin = PyList_New(origin_depth); PyFrameObject *frame = PyEval_GetFrame(); - int i = 0; - for (; frame && i < origin_depth; ++i) { + /* First count how many frames we have */ + int frame_count = 0; + for (; frame && frame_count < origin_depth; ++frame_count) { + frame = frame->f_back; + } + + /* Now collect them */ + PyObject *cr_origin = PyTuple_New(frame_count); + frame = PyEval_GetFrame(); + for (int i = 0; i < frame_count; ++i) { PyObject *frameinfo = Py_BuildValue( "OiO", frame->f_code->co_filename, @@ -1179,16 +1185,9 @@ compute_cr_origin(int origin_depth) Py_DECREF(cr_origin); return NULL; } - PyList_SET_ITEM(cr_origin, i, frameinfo); + PyTuple_SET_ITEM(cr_origin, i, frameinfo); frame = frame->f_back; } - /* Truncate the list if necessary */ - if (i < origin_depth) { - if (PyList_SetSlice(cr_origin, i, origin_depth, NULL) < 0) { - Py_DECREF(cr_origin); - return NULL; - } - } return cr_origin; }