Closing this in favor of the discussion in #733. It sounds like the plan is to switch to always raising KeyboardInterrupt-at-checkpoint in a new task, so that all existing tasks experience it as a cancellation, which can be shielded against normally.

We might be injecting it in a new task, but if we put the new task in a nursery that's surrounded by a shielded cancel scope, it will still behave idiosyncratically: the other tasks in that nursery will be cancelled due to what is effectively a top-level cancellation, even though there's a shield between them and the top level.

Reopening this issue to track that. Probably the solution is to not inject the KI task in any nursery that would not experience a top-level cancellation.

If you're good with that, then I agree it's much easier! My rationale for picking the innermost main task nursery in #1537 was that it's the most likely to match the point where current Trio raises KI, so most likely to be backwards-compatible with people trying to catch KI within Trio. If we think that doesn't matter (beyond notifying #1 and mentioning in the release notes), then let's take the simpler approach.

Eh, I wouldn't say it "doesn't matter" exactly – it would be nice if we could inject the KeyboardInterrupt inside the main task. But it introduces a ton of complicated edge cases: what if there's no main-task nursery? what if the main task has keyboard interrupt protection enabled? What if the main task is shielded? what if the user did trio.run(..., keyboard_interrupt_is_always_cancel=True)? and so on and so on...

And it's easy to justify in docs: say a KI arrives while a system task is running; why should that get diverted "sideways" into a sibling task that wasn't running? Having it come out of trio.run instead is pretty logical.

And I think the required changes to user code should be fairly minor?

So on net I think the disruption is justified. Though we'll see what users think :-)

FWIW, I tend to agree. Most likely, the interrupt occurs while Trio is waiting on epoll/select/io_uring/whatever, and if it doesn't we can just assume that it did because it's all async anyway. ;-)

If we really want to divert a KI to some task, the "main" task is the last task I'd expect the interrupt to be diverted to; a much more helpful candidate would be whichever task ran last (and didn't terminate of course), on the grounds that this is the one that's most likely to be the task of interest, from the PoV of somebody scratching their head when they squint at the traceback.

However, that (as @njsmith mentioned) has a ton of edge cases which we could of course somehow handle, but why bother? The effort would be much better spent by implementing some task monitor-ish code which, when turned on, would print all running tasks and their tracebacks. That kind of introspectability would be helpful in a lot of other cases …

I agree that there should be few or no changes to user code. You shouldn't explicitly handle KeyboardInterrupt within async code anyway, that's what BaseException is for.

The logic would be: Iterate the main task's nurseries from innermost outward. If you find a nursery in which it would be reasonable to raise KI (open, unshielded, not KI-protected), then inject the KI there. Otherwise, inject it at system task level.

It's not too complex to implement, but it definitely does create a number of additional cases to test, including the rather thorny "KI arrives during the one-tick interval between a nursery being closed and that nursery being removed from parent_task.child_nurseries". I'd just as soon go with the simpler approach if everyone is happy with the backcompat tradeoffs. But the more complex approach is feasible too (and mostly implemented already) if the complexity is thought to be worth it. :-)

Otherwise, inject it at system task level.

The thing is... The one downside of always injecting it at the system task level is that users need to move their except KeyboardInterrupt blocks out around trio.run. But... If we can sometimes inject it there, then they have to do the same thing. So if we're going to do it at all, then we might as well do it all the time.

So if we're going to do it at all, then we might as well do it all the time.

Makes sense! I've updated #1537 to take this approach.