Skip to content

Commit

Permalink
Fire and forget async ctx blocks anti-pattern
Browse files Browse the repository at this point in the history
  • Loading branch information
gsalgado committed Jun 1, 2020
1 parent 2ce3fa4 commit 0eec1f3
Showing 1 changed file with 33 additions and 0 deletions.
33 changes: 33 additions & 0 deletions antipatterns.md
Original file line number Diff line number Diff line change
Expand Up @@ -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()
```

0 comments on commit 0eec1f3

Please sign in to comment.