Fire and forget async ctx blocks anti-pattern

Author: gsalgado

antipatterns.md

@@ -164,3 +164,36 @@ class User:
 Replacing `assert` statements with `raise AssertionError(...)` (or whatever
 exception class you prefer) ensures that these checks cannot be trivially
 disabled.
+
+
+## Fire and forget async context blocks
+
+When writing asyncio-based async context blocks (i.e. using async context managers), we sometimes
+fail to continuously check that the background task started is still running. For example, the
+following is how our Connection service was implemented
+(https://github.com/ethereum/trinity/blob/0db2a36706e5327fa040258bb5fef3fae75d9d8c/p2p/connection.py#L132-L153)
+and since it doesn't perform any health checks on the multiplexing task running in the background,
+a crash in the multiplexer that failed to cancel the connection would leave the connection running
+forever.
+
+```python
+async def run():
+    async with self._multiplexer.multiplex():
+        ...
+        await self.manager.wait_finished()
+```
+
+In order to avoid this we need to make sure our async context managers always yield a reference
+to something that can be used to wait for (or check the state) of the background task. In the
+above example it could be something like:
+
+```python
+async def run():
+    async with self._multiplexer.multiplex() as multiplexing_task:
+        ...
+        await asyncio.wait(
+            self.manager.wait_finished(), multiplexing_task,
+            return_when=asyncio.FIST_COMPLETED)
+        if multiplexing_task.done():
+            self.manager.cancel()
+```