gimlet-seq: don't ignore notifications in dispatch #1627
Merged
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
I introduced a regression in the `gimlet-seq` task in commit 4cb9edd, where the call to `idol_runtime::dispatch_n` was inadvertently replaced with `idol_runtime::dispatch`. This resulted in the `gimlet-seq` task ignoring notifications that it previously handled. In particular, the bug causes the `gimlet-seq` task to ignore the timer notifications it sets for itself to poll the sequencer state after reaching A0. This manifests as a failure to power on the T6, as described in issue #1625. This commit fixes the regression by changing `dispatch` back to `dispatch_n`. Thanks to @wesolows for identifying the root cause of this bug! I've tested this on Gimlet c71 in the lab, and after applying this patch, the T6 once again powers on as expected: ```console BRM42220071 # svcs t6init STATE STIME FMRI online 0:02:53 svc:/system/t6init:default ``` Fixes #1625 Authored-by: Keith M Wesolowski <60712355+wesolows@users.noreply.github.com>
This was referenced Feb 22, 2024
wesolows
approved these changes
Feb 22, 2024
There was a problem hiding this comment.
LGTM. I would like to suggest a followup, perhaps in idolatry, to make this kind of error impossible. My suggestion of a negative trait bound on S in dispatch was somewhat flippant but perhaps there is a practical means of achieving the same thing (which I wouldn't know).
Yeah, I think @cbiffle is looking into this! It's definitely unfortunate that a two-character function name difference can leave a task permanently stuck... |
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.
I introduced a regression in the
gimlet-seqtask in commit 4cb9edd, where the call toidol_runtime::dispatch_nwas inadvertently replaced withidol_runtime::dispatch. This resulted in thegimlet-seqtask ignoring notifications that it previously handled. In particular, the bug causes thegimlet-seqtask to ignore the timer notifications it sets for itself to poll the sequencer state after reaching A0. This manifests as a failure to power on the T6, as described in issue #1625.This commit fixes the regression by changing
dispatchback todispatch_n. Thanks to @wesolows for identifying the root cause of this bug!I've tested this on Gimlet c71 in the lab, and after applying this patch, the T6 once again powers on as expected:
Fixes #1625