The current Synchronizer._run_function_sync() implementation relies on run_coroutine_threadsafe which returns a concurrent.futures.Future that we then wait for blockingly using .result()
However, if a KeyboardInterrupt (or any other "external" exception, i.e. not from the called async code) stops the .result() call, the underlying async call in the synchronicity event loop/thread isn't cancelled. That won't happen until the synchronizer teardown which is much later in the lifecycle.
This naturally also means any context managers surrounding the wrapped call would have their exit handlers trigger before the called code is cancelled, e.g.
with modal.enable_output():
run_app() # <- ctrl C during this call would tear down the output manager before the app can write to it!
My first idea was to fix this by simply running fut.cancel() on the concurrent.futures.Future on KeyboardInterrupt, but that wouldn't leave is with a way to wait for potential cancellation handling within the coroutine to complete before reraising in the main thread (so there would be a race between the threads on handling of the error).
This patch instead fixes the issue through some semi-hacky two-way communication with the called coroutine, by means of an asyncio.Event and a asyncio.wait(), that tracks interruption of the blocking result() call 😵
The current
Synchronizer._run_function_sync()
implementation relies onrun_coroutine_threadsafe
which returns a concurrent.futures.Future that we then wait for blockingly using.result()
However, if a
KeyboardInterrupt
(or any other "external" exception, i.e. not from the called async code) stops the.result()
call, the underlying async call in the synchronicity event loop/thread isn't cancelled. That won't happen until the synchronizer teardown which is much later in the lifecycle.This means that:
Will print:
instead of
This naturally also means any context managers surrounding the wrapped call would have their exit handlers trigger before the called code is cancelled, e.g.
My first idea was to fix this by simply running
fut.cancel()
on the concurrent.futures.Future on KeyboardInterrupt, but that wouldn't leave is with a way to wait for potential cancellation handling within the coroutine to complete before reraising in the main thread (so there would be a race between the threads on handling of the error).This patch instead fixes the issue through some semi-hacky two-way communication with the called coroutine, by means of an asyncio.Event and a
asyncio.wait()
, that tracks interruption of the blockingresult()
call 😵