Skip to content

bpo-33672: Fix Task.__repr__ crash with Cython's bogus coroutines #7161

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
May 28, 2018
Merged
Show file tree
Hide file tree
Changes from all 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
81 changes: 45 additions & 36 deletions Lib/asyncio/coroutines.py
Original file line number Diff line number Diff line change
Expand Up @@ -189,66 +189,75 @@ def iscoroutine(obj):
def _format_coroutine(coro):
assert iscoroutine(coro)

if not hasattr(coro, 'cr_code') and not hasattr(coro, 'gi_code'):
# Most likely a built-in type or a Cython coroutine.

# Built-in types might not have __qualname__ or __name__.
coro_name = getattr(
coro, '__qualname__',
getattr(coro, '__name__', type(coro).__name__))
coro_name = f'{coro_name}()'
is_corowrapper = isinstance(coro, CoroWrapper)

def get_name(coro):
# Coroutines compiled with Cython sometimes don't have
# proper __qualname__ or __name__. While that is a bug
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you have any insights into the conditions under which this can be observed? Is this a problem with older versions of Cython, or is there anything we can do to improve the situation?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure about __qualname__ and __name__, I got reports from uvloop users about this a couple of months ago. __code__ set to None should be easily reproducible with latest Cython -- ideally, __code__ should either be set to a code-like object or not set at all.

# in Cython, asyncio shouldn't crash with an AttributeError
# in its __repr__ functions.
if is_corowrapper:
return format_helpers._format_callback(coro.func, (), {})

if hasattr(coro, '__qualname__') and coro.__qualname__:
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't follow why not push all this logic into format_helpers._format_callback?

Copy link
Member Author

Choose a reason for hiding this comment

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

format_callback expects a Python function. This works on a coroutine object. This whole function should be moved to format_helpers.py though, I just don't want to do this in 3.7

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, got it.

coro_name = coro.__qualname__
elif hasattr(coro, '__name__') and coro.__name__:
coro_name = coro.__name__
else:
# Stop masking Cython bugs, expose them in a friendly way.
coro_name = f'<{type(coro).__name__} without __name__>'
return f'{coro_name}()'

running = False
def is_running(coro):
try:
running = coro.cr_running
return coro.cr_running
except AttributeError:
try:
running = coro.gi_running
return coro.gi_running
except AttributeError:
pass
return False

if running:
coro_code = None
if hasattr(coro, 'cr_code') and coro.cr_code:
coro_code = coro.cr_code
elif hasattr(coro, 'gi_code') and coro.gi_code:
coro_code = coro.gi_code

coro_name = get_name(coro)

if not coro_code:
# Built-in types might not have __qualname__ or __name__.
if is_running(coro):
return f'{coro_name} running'
else:
return coro_name

coro_name = None
if isinstance(coro, CoroWrapper):
func = coro.func
coro_name = coro.__qualname__
if coro_name is not None:
coro_name = f'{coro_name}()'
else:
func = coro

if coro_name is None:
coro_name = format_helpers._format_callback(func, (), {})

try:
coro_code = coro.gi_code
except AttributeError:
coro_code = coro.cr_code

try:
coro_frame = None
if hasattr(coro, 'gi_frame') and coro.gi_frame:
coro_frame = coro.gi_frame
except AttributeError:
elif hasattr(coro, 'cr_frame') and coro.cr_frame:
coro_frame = coro.cr_frame

filename = coro_code.co_filename
# If Cython's coroutine has a fake code object without proper
# co_filename -- expose that.
filename = coro_code.co_filename or '<empty co_filename>'

lineno = 0
if (isinstance(coro, CoroWrapper) and
not inspect.isgeneratorfunction(coro.func) and
coro.func is not None):
if (is_corowrapper and
coro.func is not None and
not inspect.isgeneratorfunction(coro.func)):
source = format_helpers._get_function_source(coro.func)
if source is not None:
filename, lineno = source
if coro_frame is None:
coro_repr = f'{coro_name} done, defined at {filename}:{lineno}'
else:
coro_repr = f'{coro_name} running, defined at {filename}:{lineno}'

elif coro_frame is not None:
lineno = coro_frame.f_lineno
coro_repr = f'{coro_name} running at {filename}:{lineno}'

else:
lineno = coro_code.co_firstlineno
coro_repr = f'{coro_name} done, defined at {filename}:{lineno}'
Expand Down
9 changes: 5 additions & 4 deletions Lib/asyncio/format_helpers.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import functools
import inspect
import reprlib
import sys
import traceback

from . import constants
Expand Down Expand Up @@ -45,10 +46,10 @@ def _format_callback(func, args, kwargs, suffix=''):
suffix = _format_args_and_kwargs(args, kwargs) + suffix
return _format_callback(func.func, func.args, func.keywords, suffix)

if hasattr(func, '__qualname__'):
func_repr = getattr(func, '__qualname__')
elif hasattr(func, '__name__'):
func_repr = getattr(func, '__name__')
if hasattr(func, '__qualname__') and func.__qualname__:
func_repr = func.__qualname__
elif hasattr(func, '__name__') and func.__name__:
func_repr = func.__name__
else:
func_repr = repr(func)

Expand Down
10 changes: 10 additions & 0 deletions Lib/test/test_asyncio/test_events.py
Original file line number Diff line number Diff line change
Expand Up @@ -2784,11 +2784,21 @@ def test_coroutine_like_object_debug_formatting(self):
coro.cr_running = True
self.assertEqual(coroutines._format_coroutine(coro), 'BBB() running')

coro.__name__ = coro.__qualname__ = None
self.assertEqual(coroutines._format_coroutine(coro),
'<CoroLike without __name__>() running')

coro = CoroLike()
coro.__qualname__ = 'CoroLike'
# Some coroutines might not have '__name__', such as
# built-in async_gen.asend().
self.assertEqual(coroutines._format_coroutine(coro), 'CoroLike()')

coro = CoroLike()
coro.__qualname__ = 'AAA'
coro.cr_code = None
self.assertEqual(coroutines._format_coroutine(coro), 'AAA()')


class TimerTests(unittest.TestCase):

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix Task.__repr__ crash with Cython's bogus coroutines