Skip to content
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions Doc/library/inspect.rst
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,10 @@ attributes:
+-----------+-------------------+---------------------------+
| | cr_code | code |
+-----------+-------------------+---------------------------+
| | cr_origin | where coroutine was |
| | | created, if coroutine |
| | | origin tracking is enabled|
+-----------+-------------------+---------------------------+

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a link to the set_coroutine_origin_tracking_depth documentation snippet?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, that was my original thought. The problem is that

:func:`set_coroutine_origin_tracking_depth`

is too long to fit in the ascii-art box. So... either we need a shorter name for the function, or we need to redraw this whole giant table, and I couldn't think of a satisfactory way to do either in the 2 minutes I spent thinking about it :-). Any suggestions?

I guess we could use that weird ReST substitution thing? I'm not sure how that works.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

| builtin | __doc__ | documentation string |
+-----------+-------------------+---------------------------+
| | __name__ | original name of this |
Expand All @@ -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])

Expand Down
21 changes: 21 additions & 0 deletions Doc/library/sys.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to specify how the list is ordered.


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.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thread-local -> thread-specific


.. 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)

Expand Down
1 change: 1 addition & 0 deletions Include/ceval.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 *);
Expand Down
1 change: 1 addition & 0 deletions Include/genobject.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 2 additions & 0 deletions Include/pystate.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
4 changes: 4 additions & 0 deletions Include/warnings.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);

@1st1 1st1 Jan 20, 2018 •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Strictly speaking we don't need PyAPI_FUNC here, as warnings.c|h will be linked with genobject.c and we are not going to use it in extensions.

#endif

#ifdef __cplusplus
}
#endif
Expand Down
49 changes: 16 additions & 33 deletions Lib/asyncio/base_events.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
from . import sslproto
from . import tasks
from .log import logger
from .constants import DEBUG_STACK_DEPTH

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In asyncio we always import modules, so please change to from . import constants.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

like in the line above, from .log import logger? :-)

(yes I fixed it)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That logger import is probably the only offender, will fix it in a separate PR :)



__all__ = 'BaseEventLoop',
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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()
Expand All @@ -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)

Expand Down Expand Up @@ -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:

@1st1 1st1 Jan 20, 2018 •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if enabled and self._coroutine_origin_tracking_enabled:?

I don't like using == for bools.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, that would be different – the no-== equivalent would be:

if ((enabled and self._coroutine_origin_tracking_enabled) or (not enabled and not self._coroutine_origin_tracking_enabled)):
   ...

I find the == version easier to read. ("If the requested state equals the current state, return without doing anything.")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it, then I'd cast them both to bool: if bool(enabled) == bool(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
)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please move the closing paren to the prev line:

sys.set_coroutine_origin_tracking_depth(
    self._coroutine_origin_tracking_saved_depth)

(that's just the code style in asyncio package that I'd like to maintain for consistency; and I'm slowly fixing places where it's different)


self._coroutine_origin_tracking_enabled = enabled

def get_debug(self):
return self._debug
Expand All @@ -1572,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)
10 changes: 0 additions & 10 deletions Lib/asyncio/coroutines.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down Expand Up @@ -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.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can also remove cr_* properties from CoroWrapper.

return func

if inspect.isgeneratorfunction(func):
Expand Down
39 changes: 12 additions & 27 deletions Lib/test/test_asyncio/test_pep492.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
"""Tests support for new syntax introduced by PEP 492."""

import sys
import types
import unittest

Expand Down Expand Up @@ -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'<CoroWrapper .*\.foo\(\) running at .*pep492.*>')

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():
Expand Down Expand Up @@ -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())

Expand Down
Loading