Sync traps #103
Merged
Sync traps #103
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Adds a SyncTraps enum, and refactors various bits of the trap code to simplify it in general -- in particular, drops the useless 0th argument to every trap. Doesn't yet contain any code to call a sync trap, or any sync trap implementations.
And convert _trap_adjust_cancel_depth to use the new calling mechanism, as a kind of proof-of-concept.
This covers all the obvious ones. There are a few that could become sync traps, but where things are a bit more complicated: - _trap_cancel_task is synchronous in most cases... except that sometimes it yields while waiting for the other task to block on a cancellation point. Probably this should be fixed - _trap_reschedule_tasks has a synchronous kind of feel -- it marks some other task(s) as runnable, and never needs to block. But there are tests that fail if we return immediately without rescheduling. - _trap_spawn: it could be synchronous, but there are a few options here and it's not clear which is better. Options include at least: (a) Always return immediately. (b) Always run the child task until its first blocking point before resuming the parent task. (c) Don't provide any guarantees any way. So we'll defer those for now...
Also add docs for some undocumented traps
See test case -- you'd expect sleep(0) to yield the event loop and give other things a chance to run, but it didn't quite work before.
|
Just did the poor-form thing and added some small cleanups and an only-semi-related bug fix here... I can split them off if needed. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Refactor trap internals to formalize the distinction between regular and "synchronous" traps. See #101 for motivation. Also convert the "obvious" traps to make them sync traps.
Specifically, the following traps are made sync:
adjust_cancel_defer_depthget_kernelget_currentset_timeoutunset_timeoutqueue_reschedule_functionclocksigwatchsigunwatchThe following traps in principle could be made sync, but either there's some complication or it isn't obvious whether those are actually the semantics we want, so I left them alone for now:
cancel_taskspawnreschedule_tasksWhen switching traps to make them synchronous I also renamed them. This isn't at all mandatory, but since the name of the trap = the name of the trap handler function, and sync trap handlers have a different calling convention than regular trap handlers, I thought it was helpful to have that reminder in the name of which kind of handler we're implementing.
This also contains some small cleanups to regular trap handling and adds some missing docs.