[Python-checkins] bpo-39606: allow closing async generators that are already closed (GH-18475) (GH-18502)

[Nathaniel J. Smith]

https://github.com/python/cpython/commit/f464edf3239f7867fe31c9cd238a68fb3b90feaa
commit: f464edf3239f7867fe31c9cd238a68fb3b90feaa
branch: 3.7
author: Nathaniel J. Smith <njs at pobox.com>
committer: GitHub <noreply at github.com>
date: 2020-02-13T01:33:35-08:00
summary:

bpo-39606: allow closing async generators that are already closed (GH-18475) (GH-18502)

The fix for [bpo-39386](https://bugs.python.org/issue39386) attempted to make it so you couldn't reuse a
agen.aclose() coroutine object. It accidentally also prevented you
from calling aclose() at all on an async generator that was already
closed or exhausted. This commit fixes it so we're only blocking the
actually illegal cases, while allowing the legal cases.

The new tests failed before this patch. Also confirmed that this fixes
the test failures we were seeing in Trio with Python dev builds:
  https://github.com/python-trio/trio/pull/1396

https://bugs.python.org/issue39606
(cherry picked from commit 925dc7fb1d0db85dc137afa4cd14211bf0d67414)

files:
A Misc/NEWS.d/next/Core and Builtins/2020-02-11-23-59-07.bpo-39606.a72Sxc.rst
M Lib/test/test_asyncgen.py
M Objects/genobject.c

diff --git a/Lib/test/test_asyncgen.py b/Lib/test/test_asyncgen.py
index 426d5d1161423..5a292fb5d64e1 100644
--- a/Lib/test/test_asyncgen.py
+++ b/Lib/test/test_asyncgen.py
@@ -1117,7 +1117,7 @@ def exception_handler(loop, context):
 
         self.assertEqual([], messages)
 
-    def test_async_gen_await_anext_twice(self):
+    def test_async_gen_await_same_anext_coro_twice(self):
         async def async_iterate():
             yield 1
             yield 2
@@ -1136,7 +1136,7 @@ def test_async_gen_await_anext_twice(self):
 
         self.loop.run_until_complete(run())
 
-    def test_async_gen_await_aclose_twice(self):
+    def test_async_gen_await_same_aclose_coro_twice(self):
         async def async_iterate():
             yield 1
             yield 2
@@ -1153,6 +1153,32 @@ def test_async_gen_await_aclose_twice(self):
 
         self.loop.run_until_complete(run())
 
+    def test_async_gen_aclose_twice_with_different_coros(self):
+        # Regression test for https://bugs.python.org/issue39606
+        async def async_iterate():
+            yield 1
+            yield 2
+
+        async def run():
+            it = async_iterate()
+            await it.aclose()
+            await it.aclose()
+
+        self.loop.run_until_complete(run())
+
+    def test_async_gen_aclose_after_exhaustion(self):
+        # Regression test for https://bugs.python.org/issue39606
+        async def async_iterate():
+            yield 1
+            yield 2
+
+        async def run():
+            it = async_iterate()
+            async for _ in it:
+                pass
+            await it.aclose()
+
+        self.loop.run_until_complete(run())
 
 if __name__ == "__main__":
     unittest.main()
diff --git a/Misc/NEWS.d/next/Core and Builtins/2020-02-11-23-59-07.bpo-39606.a72Sxc.rst b/Misc/NEWS.d/next/Core and Builtins/2020-02-11-23-59-07.bpo-39606.a72Sxc.rst
new file mode 100644
index 0000000000000..b7cbe4e91f59c
--- /dev/null
+++ b/Misc/NEWS.d/next/Core and Builtins/2020-02-11-23-59-07.bpo-39606.a72Sxc.rst	
@@ -0,0 +1,2 @@
+Fix regression caused by fix for bpo-39386, that prevented calling
+``aclose`` on an async generator that had already been closed or exhausted.
diff --git a/Objects/genobject.c b/Objects/genobject.c
index a42169f82e4d7..dd7d44bd427df 100644
--- a/Objects/genobject.c
+++ b/Objects/genobject.c
@@ -1804,14 +1804,19 @@ async_gen_athrow_send(PyAsyncGenAThrow *o, PyObject *arg)
     PyFrameObject *f = gen->gi_frame;
     PyObject *retval;
 
-    if (f == NULL || f->f_stacktop == NULL ||
-            o->agt_state == AWAITABLE_STATE_CLOSED) {
+    if (o->agt_state == AWAITABLE_STATE_CLOSED) {
         PyErr_SetString(
             PyExc_RuntimeError,
             "cannot reuse already awaited aclose()/athrow()");
         return NULL;
     }
 
+    if (f == NULL || f->f_stacktop == NULL) {
+        o->agt_state = AWAITABLE_STATE_CLOSED;
+        PyErr_SetNone(PyExc_StopIteration);
+        return NULL;
+    }
+
     if (o->agt_state == AWAITABLE_STATE_INIT) {
         if (o->agt_gen->ag_closed) {
             PyErr_SetNone(PyExc_StopIteration);
@@ -1882,6 +1887,7 @@ async_gen_athrow_send(PyAsyncGenAThrow *o, PyObject *arg)
     }
 
 yield_close:
+    o->agt_state = AWAITABLE_STATE_CLOSED;
     PyErr_SetString(
         PyExc_RuntimeError, ASYNC_GEN_IGNORED_EXIT_MSG);
     return NULL;
@@ -1924,6 +1930,7 @@ async_gen_athrow_throw(PyAsyncGenAThrow *o, PyObject *args)
     } else {
         /* aclose() mode */
         if (retval && _PyAsyncGenWrappedValue_CheckExact(retval)) {
+            o->agt_state = AWAITABLE_STATE_CLOSED;
             Py_DECREF(retval);
             PyErr_SetString(PyExc_RuntimeError, ASYNC_GEN_IGNORED_EXIT_MSG);
             return NULL;