[Python-checkins] bpo-33672: Fix Task.__repr__ crash with Cython's bogus coroutines (GH-7161)

[Yury Selivanov]

https://github.com/python/cpython/commit/989b9e0e6d7dd2fa911f9bfd4744e7f3a82d6006
commit: 989b9e0e6d7dd2fa911f9bfd4744e7f3a82d6006
branch: master
author: Yury Selivanov <yury at magic.io>
committer: GitHub <noreply at github.com>
date: 2018-05-28T16:27:34-04:00
summary:

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

files:
A Misc/NEWS.d/next/Library/2018-05-28-12-29-54.bpo-33672.GM_Xm_.rst
M Lib/asyncio/coroutines.py
M Lib/asyncio/format_helpers.py
M Lib/test/test_asyncio/test_events.py

diff --git a/Lib/asyncio/coroutines.py b/Lib/asyncio/coroutines.py
index c7fcd4425584..c665ebe33ee1 100644
--- a/Lib/asyncio/coroutines.py
+++ b/Lib/asyncio/coroutines.py
@@ -189,56 +189,63 @@ 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
+        # 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__:
+            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
@@ -246,9 +253,11 @@ def _format_coroutine(coro):
             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}'
diff --git a/Lib/asyncio/format_helpers.py b/Lib/asyncio/format_helpers.py
index 39cfcee0c1c8..27d11fd4fa95 100644
--- a/Lib/asyncio/format_helpers.py
+++ b/Lib/asyncio/format_helpers.py
@@ -1,6 +1,7 @@
 import functools
 import inspect
 import reprlib
+import sys
 import traceback
 
 from . import constants
@@ -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)
 
diff --git a/Lib/test/test_asyncio/test_events.py b/Lib/test/test_asyncio/test_events.py
index d7b0a665a0ab..ba28e8ce875c 100644
--- a/Lib/test/test_asyncio/test_events.py
+++ b/Lib/test/test_asyncio/test_events.py
@@ -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):
 
diff --git a/Misc/NEWS.d/next/Library/2018-05-28-12-29-54.bpo-33672.GM_Xm_.rst b/Misc/NEWS.d/next/Library/2018-05-28-12-29-54.bpo-33672.GM_Xm_.rst
new file mode 100644
index 000000000000..36373c028639
--- /dev/null
+++ b/Misc/NEWS.d/next/Library/2018-05-28-12-29-54.bpo-33672.GM_Xm_.rst
@@ -0,0 +1 @@
+Fix Task.__repr__ crash with Cython's bogus coroutines