-
-
Notifications
You must be signed in to change notification settings - Fork 341
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Raise deferred KeyboardInterrupt out of run() after cancelling all tasks #1537
base: main
Are you sure you want to change the base?
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1537 +/- ##
==========================================
- Coverage 99.67% 99.67% -0.01%
==========================================
Files 108 108
Lines 13358 13345 -13
Branches 1012 1008 -4
==========================================
- Hits 13315 13302 -13
Misses 28 28
Partials 15 15
|
The coverage "failure" is because |
Sorry for the merge conflict, I just merged the Black pull request |
I'm not sure cancelling the system nursery is a great idea... until now, we had an invariant that system tasks would only be cancelled (a) after the main task exited, or (b) after a system task crashed. So they basically had the "service task" semantics discussed in #1521. I feel like |
That makes sense to me, but I think it will be easiest to implement by just doing #1521, because I don't see any way to wrap the main task in a dedicated cancel scope without adding another frame to every Trio traceback. |
Maybe possibly we could do something like this? async def init(self, async_fn, args):
async with open_nursery() as system_nursery:
self.system_nursery = system_nursery
async with open_nursery() as main_task_nursery: ##### <-----
try:
self.main_task = self.spawn_impl(async_fn, args, main_task_nursery, None)
except BaseException as exc:
self.main_task_outcome = Error(exc)
system_nursery.cancel_scope.cancel()
self.entry_queue.spawn() but yeah, I see your point |
Fixes #151, first step of #733.