[Python-checkins] bpo-41229: Update docs for explicit aclose()-required cases and add contextlib.aclosing() method (GH-21545)

[miss-islington]

https://github.com/python/cpython/commit/6e8dcdaaa49d4313bf9fab9f9923ca5828fbb10e
commit: 6e8dcdaaa49d4313bf9fab9f9923ca5828fbb10e
branch: master
author: Joongi Kim <joongi at lablup.com>
committer: miss-islington <31488909+miss-islington at users.noreply.github.com>
date: 2020-11-02T00:02:48-08:00
summary:

bpo-41229: Update docs for explicit aclose()-required cases and add contextlib.aclosing() method (GH-21545)



This is a PR to:

 * Add `contextlib.aclosing` which ia analogous to `contextlib.closing` but for async-generators with an explicit test case for [bpo-41229]()
 * Update the docs to describe when we need explicit `aclose()` invocation.

which are motivated by the following issues, articles, and examples:

 * [bpo-41229]()
 * https://github.com/njsmith/async_generator
 * https://vorpus.org/blog/some-thoughts-on-asynchronous-api-design-in-a-post-asyncawait-world/#cleanup-in-generators-and-async-generators
 * https://www.python.org/dev/peps/pep-0533/
 * https://github.com/achimnol/aiotools/blob/ef7bf0cea7af/src/aiotools/context.py#L152

Particuarly regarding [PEP-533](https://www.python.org/dev/peps/pep-0533/), its acceptance (`__aiterclose__()`) would make this little addition of `contextlib.aclosing()` unnecessary for most use cases, but until then this could serve as a good counterpart and analogy to `contextlib.closing()`. The same applies for `contextlib.closing` with `__iterclose__()`.
Also, still there are other use cases, e.g., when working with non-generator objects with `aclose()` methods.

files:
A Misc/NEWS.d/next/Library/2020-07-19-20-10-41.bpo-41229.p8rJa2.rst
M Doc/library/contextlib.rst
M Doc/reference/expressions.rst
M Lib/contextlib.py
M Lib/test/test_contextlib_async.py

diff --git a/Doc/library/contextlib.rst b/Doc/library/contextlib.rst
index 0aa4ad7652348..e42f5a9328166 100644
--- a/Doc/library/contextlib.rst
+++ b/Doc/library/contextlib.rst
@@ -154,6 +154,39 @@ Functions and classes provided:
    ``page.close()`` will be called when the :keyword:`with` block is exited.
 
 
+.. class:: aclosing(thing)
+
+   Return an async context manager that calls the ``aclose()`` method of *thing*
+   upon completion of the block.  This is basically equivalent to::
+
+      from contextlib import asynccontextmanager
+
+      @asynccontextmanager
+      async def aclosing(thing):
+          try:
+              yield thing
+          finally:
+              await thing.aclose()
+
+   Significantly, ``aclosing()`` supports deterministic cleanup of async
+   generators when they happen to exit early by :keyword:`break` or an
+   exception.  For example::
+
+      from contextlib import aclosing
+
+      async with aclosing(my_generator()) as values:
+          async for value in values:
+              if value == 42:
+                  break
+
+   This pattern ensures that the generator's async exit code is executed in
+   the same context as its iterations (so that exceptions and context
+   variables work as expected, and the exit code isn't run after the
+   lifetime of some task it depends on).
+
+   .. versionadded:: 3.10
+
+
 .. _simplifying-support-for-single-optional-context-managers:
 
 .. function:: nullcontext(enter_result=None)
diff --git a/Doc/reference/expressions.rst b/Doc/reference/expressions.rst
index 512aa5af95619..8ac626444843d 100644
--- a/Doc/reference/expressions.rst
+++ b/Doc/reference/expressions.rst
@@ -643,6 +643,16 @@ after resuming depends on the method which resumed the execution.  If
 :meth:`~agen.asend` is used, then the result will be the value passed in to
 that method.
 
+If an asynchronous generator happens to exit early by :keyword:`break`, the caller
+task being cancelled, or other exceptions, the generator's async cleanup code
+will run and possibly raise exceptions or access context variables in an
+unexpected context--perhaps after the lifetime of tasks it depends, or
+during the event loop shutdown when the async-generator garbage collection hook
+is called.
+To prevent this, the caller must explicitly close the async generator by calling
+:meth:`~agen.aclose` method to finalize the generator and ultimately detach it
+from the event loop.
+
 In an asynchronous generator function, yield expressions are allowed anywhere
 in a :keyword:`try` construct. However, if an asynchronous generator is not
 resumed before it is finalized (by reaching a zero reference count or by
@@ -654,9 +664,9 @@ generator-iterator's :meth:`~agen.aclose` method and run the resulting
 coroutine object, thus allowing any pending :keyword:`!finally` clauses
 to execute.
 
-To take care of finalization, an event loop should define
-a *finalizer* function which takes an asynchronous generator-iterator
-and presumably calls :meth:`~agen.aclose` and executes the coroutine.
+To take care of finalization upon event loop termination, an event loop should
+define a *finalizer* function which takes an asynchronous generator-iterator and
+presumably calls :meth:`~agen.aclose` and executes the coroutine.
 This  *finalizer* may be registered by calling :func:`sys.set_asyncgen_hooks`.
 When first iterated over, an asynchronous generator-iterator will store the
 registered *finalizer* to be called upon finalization. For a reference example
diff --git a/Lib/contextlib.py b/Lib/contextlib.py
index ff92d9f913f4c..82ddc1497d863 100644
--- a/Lib/contextlib.py
+++ b/Lib/contextlib.py
@@ -303,6 +303,32 @@ def __exit__(self, *exc_info):
         self.thing.close()
 
 
+class aclosing(AbstractAsyncContextManager):
+    """Async context manager for safely finalizing an asynchronously cleaned-up
+    resource such as an async generator, calling its ``aclose()`` method.
+
+    Code like this:
+
+        async with aclosing(<module>.fetch(<arguments>)) as agen:
+            <block>
+
+    is equivalent to this:
+
+        agen = <module>.fetch(<arguments>)
+        try:
+            <block>
+        finally:
+            await agen.aclose()
+
+    """
+    def __init__(self, thing):
+        self.thing = thing
+    async def __aenter__(self):
+        return self.thing
+    async def __aexit__(self, *exc_info):
+        await self.thing.aclose()
+
+
 class _RedirectStream(AbstractContextManager):
 
     _stream = None
diff --git a/Lib/test/test_contextlib_async.py b/Lib/test/test_contextlib_async.py
index 43fb7fced1bfd..3765f6cbf28c5 100644
--- a/Lib/test/test_contextlib_async.py
+++ b/Lib/test/test_contextlib_async.py
@@ -1,5 +1,5 @@
 import asyncio
-from contextlib import asynccontextmanager, AbstractAsyncContextManager, AsyncExitStack
+from contextlib import aclosing, asynccontextmanager, AbstractAsyncContextManager, AsyncExitStack
 import functools
 from test import support
 import unittest
@@ -279,6 +279,63 @@ async def woohoo(self, func, args, kwds):
             self.assertEqual(target, (11, 22, 33, 44))
 
 
+class AclosingTestCase(unittest.TestCase):
+
+    @support.requires_docstrings
+    def test_instance_docs(self):
+        cm_docstring = aclosing.__doc__
+        obj = aclosing(None)
+        self.assertEqual(obj.__doc__, cm_docstring)
+
+    @_async_test
+    async def test_aclosing(self):
+        state = []
+        class C:
+            async def aclose(self):
+                state.append(1)
+        x = C()
+        self.assertEqual(state, [])
+        async with aclosing(x) as y:
+            self.assertEqual(x, y)
+        self.assertEqual(state, [1])
+
+    @_async_test
+    async def test_aclosing_error(self):
+        state = []
+        class C:
+            async def aclose(self):
+                state.append(1)
+        x = C()
+        self.assertEqual(state, [])
+        with self.assertRaises(ZeroDivisionError):
+            async with aclosing(x) as y:
+                self.assertEqual(x, y)
+                1 / 0
+        self.assertEqual(state, [1])
+
+    @_async_test
+    async def test_aclosing_bpo41229(self):
+        state = []
+
+        class Resource:
+            def __del__(self):
+                state.append(1)
+
+        async def agenfunc():
+            r = Resource()
+            yield -1
+            yield -2
+
+        x = agenfunc()
+        self.assertEqual(state, [])
+        with self.assertRaises(ZeroDivisionError):
+            async with aclosing(x) as y:
+                self.assertEqual(x, y)
+                self.assertEqual(-1, await x.__anext__())
+                1 / 0
+        self.assertEqual(state, [1])
+
+
 class TestAsyncExitStack(TestBaseExitStack, unittest.TestCase):
     class SyncAsyncExitStack(AsyncExitStack):
         @staticmethod
diff --git a/Misc/NEWS.d/next/Library/2020-07-19-20-10-41.bpo-41229.p8rJa2.rst b/Misc/NEWS.d/next/Library/2020-07-19-20-10-41.bpo-41229.p8rJa2.rst
new file mode 100644
index 0000000000000..926133221d417
--- /dev/null
+++ b/Misc/NEWS.d/next/Library/2020-07-19-20-10-41.bpo-41229.p8rJa2.rst
@@ -0,0 +1,3 @@
+Add ``contextlib.aclosing`` for deterministic cleanup of async generators
+which is analogous to ``contextlib.closing`` for non-async generators.
+Patch by Joongi Kim and John Belmonte.