Skip to content

Commit

Permalink
aiorpcx: fix bug in timeout_after monkeypatch
Browse files Browse the repository at this point in the history
fixes #7952

see in particular #7952 (comment)
> 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 kyuupichan/aiorpcX#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
  • Loading branch information
SomberNight committed Aug 26, 2022
1 parent 629b0c5 commit 12e628e
Showing 1 changed file with 22 additions and 1 deletion.
23 changes: 22 additions & 1 deletion electrum/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -1360,6 +1361,7 @@ async def join(self):
# 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()
Expand All @@ -1373,7 +1375,26 @@ def mycancel(*args, **kwargs):
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):
Expand Down

0 comments on commit 12e628e

Please sign in to comment.