Implemented sharing the same event loop with AsyncSingleThreadContext#542
Implemented sharing the same event loop with AsyncSingleThreadContext#542Arfey wants to merge 1 commit into
Conversation
| ) | ||
| finally: | ||
| loop.close() | ||
|
|
There was a problem hiding this comment.
Instead of using asyncio.run, we rely on loop.run_until_complete(asyncio.run closes the event loop), which means we need to perform the additional cleanup that asyncio.run normally handles for us.
Internally, asyncio.run is implemented using asyncio.Runner. Below is a simplified version of Runner, with parts removed that are not relevant to our use case:
class Runner:
def close(self):
try:
_cancel_all_tasks(self._loop)
self._loop.run_until_complete(self._loop.shutdown_asyncgens())
self._loop.run_until_complete(
self._loop.shutdown_default_executor(constants.THREAD_JOIN_TIMEOUT))
finally:
if self._set_event_loop:
events.set_event_loop(None)
loop.close()
def run(self, coro, *, context=None):
self._loop = events.new_event_loop()
events.set_event_loop(self._loop)
task = self._loop.create_task(coro)
return self._loop.run_until_complete(task)I added everything except the set_event_loop logic. In our case, when running code via loop.run_until_complete, asyncio.get_event_loop() already returns the correct loop, so there is no need to set it explicitly. Instead, we reuse the same new_loop_wrap.
ba093e7 to
801bc5a
Compare
b2f1e1f to
da34cca
Compare
da34cca to
912c6ce
Compare
| python -m pip install --upgrade tox tox-py | ||
|
|
||
| - name: Run tox targets for ${{ matrix.python-version }} | ||
| timeout-minutes: 10 |
There was a problem hiding this comment.
Hey @Arfey. Thanks for your patience here, I've been looking at other issues.
I don't really have any problems with this: you're the only consumer at this point.
My main concern was churn. We only just added the class, at your request, and then we're immediately changing it. I wonder if it isn't better developed alongside the client code until we're sure it's stabilised, and then we can adopt (and advertise) the working version here? (Not least because otherwise you're waiting on a release here, which I don't imagine doing much more frequently, so end up using a local version anyway 🫠)
|
Hey 👋, I think if the public API doesn't change (and it doesn't), then it's not a problem for the end user. The current change just improves the class to allow using a long-running, event-loop-bound client within I'm not waiting on a release, but I also don't think holding off for years to see how it works before opening a PR is a good idea 😊 |
|
Sure... my question was "is this it, or are there going to be more changes" -- if the latter then I do tend to lean toward working out what's needed first. I'll merge this one for the upcoming release. |
|
Oh, got you 😌 To be honest, I've given up on supporting WSGI/ASGI compatibility (that's why we needed this), so I'm not sure whether more changes will follow, since I don't use it anymore. |
|
Perfect... 🫠 |

The pull request updates how the
AsyncSingleThreadContextworks inasgiref.syncso that multipleasync_to_synccalls within the same async context share the same asyncio event loop and thread. This improves consistency and correctness when running async code converted to sync repeatedly in one context.