-
-
Notifications
You must be signed in to change notification settings - Fork 378
Description
Right now the autojump clock works internally by spawning a system task that loops on wait_all_tasks_blocked.
Doing this using only existing public APIs is a neat trick, but it has some downsides:
-
If we add a "chaos scheduler" mode (Tools for finding scheduler-dependent "heisenbugs" #239), then we'll want the autojump clock to keep working even while task execution is being scrambled, which is awkward if the autojump clock uses a task
-
The code in
MockClockis pretty complex and hard to understand -
It makes shutdown much trickier. See Add support for async generator finalization #1564 where we're faffing around trying to figure out what sequence to tear things down in. You want the clock to be running until the very end, but if it's just some random anonymous task, that doesn't really work... (see also Allow start_clock() to start the very first system task; make MockClock do so #1579 for an approach that tries to solve this by jumping through hoop to make it a special task)
-
There's some awkward interaction between the autojump clock using
wait_all_tasks_blockedand users usingwait_all_tasks_blockeddirectly. We have a big orange warning box about this in the docs, and it motivates this uglytiebreakerargument towait_all_tasks_blockedthat we wouldn't need otherwise.
So we should probably make the autojump functionality something that doesn't rely on a task and is a bit more closely integrated with the run loop.
How should this work?
In general when implementing a feature like the MockClock, our first impulse is to try to do it using public APIs, because we want to leave the door open for our users to implement new similar features we haven't thought of and without asking us for permission.
In the case of MockClock, it's not really clear how much demand there is to write custom autojump functionality, or what it would need... so it's not clear what a "generic" API for this would look like.
I thought about maybe replacing the core support for wait_all_tasks_blocked with a mechanism for registering a callback to be invoked after all tasks were blocked, and then using that to implement both wait_all_tasks_blocked and MockClock. The appealing thing about this is that it would mean the runloop only has to deal with one concept of "things are idle", and not deal with the different variants of that. But it's not quite as simple as just a add_idle_callback(cushion, callback) type API, because wait_all_tasks_blocked needs to be able to cancel a wakeup, MockClock needs to adjust the autojump threshold...
I think the simplest code change would be to straight-up add an autojump_threshold attribute to Runner, where if it gets exceeded we call current_clock()._autojump(). This is so simple that it feels like the right implementation to me. The question is how to expose it.
One option would be to have a trio.lowlevel.set_clock_autojump_threshold, or something. Or put the autojump_threshold on the clock object itself, and document that as a public part of the trio.abc.Clock interface. Given that this is such a specialized feature and we have no idea how anyone else would use this though, trying to make a public API feels premature.
The other option would be to keep it a private API, that only MockClock can use. If we do this then we should also move MockClock into _core for consistency. I'm not super happy with the idea of offering this magic clock that no-one else can replicate, but I guess I'm even less happy with the alternatives, and it's not hard to add a public API later if anyone needs it. So I guess let's do that.