Skip to content
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

Should we call the abort_fn when rescheduling in general? #315

Open
njsmith opened this issue Sep 5, 2017 · 1 comment
Open

Should we call the abort_fn when rescheduling in general? #315

njsmith opened this issue Sep 5, 2017 · 1 comment

Comments

@njsmith
Copy link
Member

njsmith commented Sep 5, 2017

Currently the most fundamental pattern for blocking in trio is:

  • Register your task in one or more places where someone will eventually find it and call reschedule
  • Write an abort function that knows how to unwind those registrations so that no-one will call reschedule
  • Go to sleep

Later, someone finds the task and:

  • unwinds all its registrations so no-one else will call reschedule
  • calls reschedule

(with a few wrinkles I'm glossing over to support primitives with asynchronous cancellation like IOCP.)

Just now I found myself writing some code where the registrer/unregister step is rather involved, and I realized I didn't want to write the unwind code twice (once in the abort callback and again in the regular wakeup path). Plus there isn't even any convenient way to pass information from the code that goes to sleep to the code that wakes up. But it needed to be done that way, because the unregister+reschedule have to be done atomically.

The simplest thing was to manually pass the abort function over to the wakeup code, and then have it call it. But this is kind of annoying, and seemed like more work than necessary given that there's actually a pretty general pattern here.

A simple thing we can do is make the _abort_func callback on Task objects public. Now that Tasks are in hazmat this seems pretty reasonable in general, and ParkingLot already mutates this attribute so that's more evidence that it ought to be public. That would make it easy for code like the above to do:

    task_to_wake = ...
    task_to_wake.abort_func()
    trio.hazmat.reschedule(task_to_wake)

But... it's still kinda weird to be manually calling the abort callback from the waking code, given that you always have to unwind before rescheduling. Maybe reschedule should just... do that? Probably something like, adding another argument to the abort callback that says whether it's a cancellation or a regular wakeup, and then we call it on all wakeup paths?

I should do a pass through the trio codebase to see if this would make things simpler or more complicated.

@oremanj
Copy link
Member

oremanj commented Mar 18, 2019

The discussion on #242 has been pretty fruitful in terms of coming up with a slightly higher-level API for common cases of blocking operations. If we do wind up adopting something like that for the trio core, I think remaining direct users of abort_fn will probably mostly want to do different things when aborting vs completing anyway, which would reduce the relevance of this issue. Definitely worth leaving it open until we see what happens with #242 though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants