From 12e628e2e6eb39709361b587a87e35f291200d16 Mon Sep 17 00:00:00 2001 From: SomberNight Date: Fri, 26 Aug 2022 15:27:53 +0000 Subject: [PATCH] aiorpcx: fix bug in timeout_after monkeypatch fixes https://github.com/spesmilo/electrum/issues/7952 see in particular https://github.com/spesmilo/electrum/issues/7952#issuecomment-1227225602 > So the issue is with the aiorpcx monkey patch in util.py, as it > relies on side-effecting the asyncio.Task, and it patches Task.cancel. > However, aiohttp also uses Task.cancel for its own timeouting of the > http request, with the same Task object, and this confuses timeout_after. > Ultimately FxThread.run exits. related https://github.com/kyuupichan/aiorpcX/pull/47 --- note: I am not content at all with this monkey-patching approach, but at the same time I don't see how to properly fix this handling all edge-cases in aiorpcx. python 3.11 is finally adding an implementation of TaskGroup [0] and an async timeout context manager [1] in the asyncio module of the stdlib. Looking at the implementations, they look unfeasible to backport: much of the implementation of asyncio.Task had to be changed for them to work, and TaskGroup in particular relies on the new ExceptionGroups. Some of these edge cases we are battling with aiorpcx.curio look difficult to fix without those stdlib changes... Anyway, when we bump the min python to 3.11, I look forward to switching to that code instead, and ripping this stuff out. [0]: https://docs.python.org/3.11/library/asyncio-task.html#task-groups [1]: https://docs.python.org/3.11/library/asyncio-task.html#asyncio.timeout --- electrum/util.py | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/electrum/util.py b/electrum/util.py index 94cc06f78..5ca4385af 100644 --- a/electrum/util.py +++ b/electrum/util.py @@ -1324,6 +1324,7 @@ class OldTaskGroup(aiorpcx.TaskGroup): await group.spawn(task1()) await group.spawn(task2()) ``` + # TODO see if we can migrate to asyncio.timeout, introduced in python 3.11, and use stdlib instead of aiorpcx.curio... """ async def join(self): if self._wait is all: @@ -1360,6 +1361,7 @@ class OldTaskGroup(aiorpcx.TaskGroup): # see https://github.com/kyuupichan/aiorpcX/issues/44 # see https://github.com/aio-libs/async-timeout/issues/229 # see https://bugs.python.org/issue42130 and https://bugs.python.org/issue45098 +# TODO see if we can migrate to asyncio.timeout, introduced in python 3.11, and use stdlib instead of aiorpcx.curio... def _aiorpcx_monkeypatched_set_new_deadline(task, deadline): def timeout_task(): task._orig_cancel() @@ -1373,7 +1375,26 @@ def _aiorpcx_monkeypatched_set_new_deadline(task, deadline): task.cancel = mycancel task._deadline_handle = task._loop.call_at(deadline, timeout_task) -aiorpcx.curio._set_new_deadline = _aiorpcx_monkeypatched_set_new_deadline + +def _aiorpcx_monkeypatched_set_task_deadline(task, deadline): + ret = _aiorpcx_orig_set_task_deadline(task, deadline) + task._externally_cancelled = None + return ret + + +def _aiorpcx_monkeypatched_unset_task_deadline(task): + if hasattr(task, "_orig_cancel"): + task.cancel = task._orig_cancel + del task._orig_cancel + return _aiorpcx_orig_unset_task_deadline(task) + + +_aiorpcx_orig_set_task_deadline = aiorpcx.curio._set_task_deadline +_aiorpcx_orig_unset_task_deadline = aiorpcx.curio._unset_task_deadline + +aiorpcx.curio._set_new_deadline = _aiorpcx_monkeypatched_set_new_deadline +aiorpcx.curio._set_task_deadline = _aiorpcx_monkeypatched_set_task_deadline +aiorpcx.curio._unset_task_deadline = _aiorpcx_monkeypatched_unset_task_deadline class NetworkJobOnDefaultServer(Logger, ABC):