1 |
commit: bab11fcee344df488d2e7f444ea3711ce87669e3 |
2 |
Author: Zac Medico <zmedico <AT> gentoo <DOT> org> |
3 |
AuthorDate: Sun Mar 1 21:56:41 2020 +0000 |
4 |
Commit: Zac Medico <zmedico <AT> gentoo <DOT> org> |
5 |
CommitDate: Mon Mar 2 00:35:51 2020 +0000 |
6 |
URL: https://gitweb.gentoo.org/proj/portage.git/commit/?id=bab11fce |
7 |
|
8 |
_GeneratorTask: throw CancelledError in cancelled coroutine (bug 711174) |
9 |
|
10 |
Throw asyncio.CancelledError in a cancelled coroutine, ensuring |
11 |
that the coroutine can handle this exception in order to perform |
12 |
any necessary cleanup (like close the log file for bug 711174). |
13 |
Note that the asyncio.CancelledError will only be thrown in the |
14 |
coroutine if there's an opportunity (yield) before the generator |
15 |
raises StopIteration. |
16 |
|
17 |
Also fix the AsynchronousTask exit listener handling for |
18 |
compatibility with this new behavior. |
19 |
|
20 |
Fixes: 8074127bbc21 ("SpawnProcess: add _main coroutine") |
21 |
Bug: https://bugs.gentoo.org/711174 |
22 |
Signed-off-by: Zac Medico <zmedico <AT> gentoo.org> |
23 |
|
24 |
lib/_emerge/AsynchronousTask.py | 12 ++++++--- |
25 |
.../tests/util/futures/test_compat_coroutine.py | 29 +++++++++++++++++++--- |
26 |
lib/portage/util/futures/compat_coroutine.py | 19 ++++++++++---- |
27 |
3 files changed, 49 insertions(+), 11 deletions(-) |
28 |
|
29 |
diff --git a/lib/_emerge/AsynchronousTask.py b/lib/_emerge/AsynchronousTask.py |
30 |
index 1e9e177cb..580eef050 100644 |
31 |
--- a/lib/_emerge/AsynchronousTask.py |
32 |
+++ b/lib/_emerge/AsynchronousTask.py |
33 |
@@ -64,7 +64,7 @@ class AsynchronousTask(SlotObject): |
34 |
@returns: Future, result is self.returncode |
35 |
""" |
36 |
waiter = self.scheduler.create_future() |
37 |
- exit_listener = lambda self: waiter.set_result(self.returncode) |
38 |
+ exit_listener = lambda self: waiter.cancelled() or waiter.set_result(self.returncode) |
39 |
self.addExitListener(exit_listener) |
40 |
waiter.add_done_callback(lambda waiter: |
41 |
self.removeExitListener(exit_listener) if waiter.cancelled() else None) |
42 |
@@ -180,9 +180,15 @@ class AsynchronousTask(SlotObject): |
43 |
def removeExitListener(self, f): |
44 |
if self._exit_listeners is None: |
45 |
if self._exit_listener_stack is not None: |
46 |
- self._exit_listener_stack.remove(f) |
47 |
+ try: |
48 |
+ self._exit_listener_stack.remove(f) |
49 |
+ except ValueError: |
50 |
+ pass |
51 |
return |
52 |
- self._exit_listeners.remove(f) |
53 |
+ try: |
54 |
+ self._exit_listeners.remove(f) |
55 |
+ except ValueError: |
56 |
+ pass |
57 |
|
58 |
def _wait_hook(self): |
59 |
""" |
60 |
|
61 |
diff --git a/lib/portage/tests/util/futures/test_compat_coroutine.py b/lib/portage/tests/util/futures/test_compat_coroutine.py |
62 |
index f96aa9be5..b561c0227 100644 |
63 |
--- a/lib/portage/tests/util/futures/test_compat_coroutine.py |
64 |
+++ b/lib/portage/tests/util/futures/test_compat_coroutine.py |
65 |
@@ -57,20 +57,43 @@ class CompatCoroutineTestCase(TestCase): |
66 |
loop.run_until_complete(catching_coroutine(loop=loop))) |
67 |
|
68 |
def test_cancelled_coroutine(self): |
69 |
+ """ |
70 |
+ Verify that a coroutine can handle (and reraise) asyncio.CancelledError |
71 |
+ in order to perform any necessary cleanup. Note that the |
72 |
+ asyncio.CancelledError will only be thrown in the coroutine if there's |
73 |
+ an opportunity (yield) before the generator raises StopIteration. |
74 |
+ """ |
75 |
+ loop = asyncio.get_event_loop() |
76 |
+ ready_for_exception = loop.create_future() |
77 |
+ exception_in_coroutine = loop.create_future() |
78 |
|
79 |
@coroutine |
80 |
def cancelled_coroutine(loop=None): |
81 |
loop = asyncio._wrap_loop(loop) |
82 |
while True: |
83 |
- yield loop.create_future() |
84 |
+ task = loop.create_future() |
85 |
+ try: |
86 |
+ ready_for_exception.set_result(None) |
87 |
+ yield task |
88 |
+ except BaseException as e: |
89 |
+ # Since python3.8, asyncio.CancelledError inherits |
90 |
+ # from BaseException. |
91 |
+ task.done() or task.cancel() |
92 |
+ exception_in_coroutine.set_exception(e) |
93 |
+ raise |
94 |
+ else: |
95 |
+ exception_in_coroutine.set_result(None) |
96 |
|
97 |
- loop = asyncio.get_event_loop() |
98 |
future = cancelled_coroutine(loop=loop) |
99 |
- loop.call_soon(future.cancel) |
100 |
+ loop.run_until_complete(ready_for_exception) |
101 |
+ future.cancel() |
102 |
|
103 |
self.assertRaises(asyncio.CancelledError, |
104 |
loop.run_until_complete, future) |
105 |
|
106 |
+ self.assertRaises(asyncio.CancelledError, |
107 |
+ loop.run_until_complete, exception_in_coroutine) |
108 |
+ |
109 |
def test_cancelled_future(self): |
110 |
""" |
111 |
When a coroutine raises CancelledError, the coroutine's |
112 |
|
113 |
diff --git a/lib/portage/util/futures/compat_coroutine.py b/lib/portage/util/futures/compat_coroutine.py |
114 |
index b745fd845..54fc316fe 100644 |
115 |
--- a/lib/portage/util/futures/compat_coroutine.py |
116 |
+++ b/lib/portage/util/futures/compat_coroutine.py |
117 |
@@ -87,21 +87,29 @@ class _GeneratorTask(object): |
118 |
def __init__(self, generator, result, loop): |
119 |
self._generator = generator |
120 |
self._result = result |
121 |
+ self._current_task = None |
122 |
self._loop = loop |
123 |
result.add_done_callback(self._cancel_callback) |
124 |
loop.call_soon(self._next) |
125 |
|
126 |
def _cancel_callback(self, result): |
127 |
- if result.cancelled(): |
128 |
- self._generator.close() |
129 |
+ if result.cancelled() and self._current_task is not None: |
130 |
+ # The done callback for self._current_task invokes |
131 |
+ # _next in either case here. |
132 |
+ self._current_task.done() or self._current_task.cancel() |
133 |
|
134 |
def _next(self, previous=None): |
135 |
+ self._current_task = None |
136 |
if self._result.cancelled(): |
137 |
if previous is not None: |
138 |
# Consume exceptions, in order to avoid triggering |
139 |
# the event loop's exception handler. |
140 |
previous.cancelled() or previous.exception() |
141 |
- return |
142 |
+ |
143 |
+ # This will throw asyncio.CancelledError in the coroutine if |
144 |
+ # there's an opportunity (yield) before the generator raises |
145 |
+ # StopIteration. |
146 |
+ previous = self._result |
147 |
try: |
148 |
if previous is None: |
149 |
future = next(self._generator) |
150 |
@@ -124,5 +132,6 @@ class _GeneratorTask(object): |
151 |
if not self._result.cancelled(): |
152 |
self._result.set_exception(e) |
153 |
else: |
154 |
- future = asyncio.ensure_future(future, loop=self._loop) |
155 |
- future.add_done_callback(self._next) |
156 |
+ self._current_task = asyncio.ensure_future(future, loop=self._loop) |
157 |
+ self._current_task.add_done_callback(self._next) |
158 |
+ |