fix(subscriber): bail rather than panic when encountering clock skew #287
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
## Motivation Currently, if compiled in debug mode, the `console-subscriber` crate's `PollStats::end_poll` function will panic if a poll duration is negative (i.e. if the poll's end timestamp occurred before its start timestamp). Because the console currently uses non-monotonic `SystemTime`s rather than monotonic `Instant`s (due to serialization requirements), this means we are potentially succeptible to clock skew adjustments. If one occurs during a poll, and the system clock goes backwards, we may panic here. This isn't great! ## Solution This branch fixes the panic by removing the assertions, and changing `end_poll` to simply bail and print a warning rather than panicking when encountering a negative poll duration. This is a short-term solution; a better long-term solution would be to change `console-subscriber` to use monotonic `Instant`s rather than non-monotonic `SystemTime`s (as discussed [here][1] and [here][2]). Fixes #286 [1]:#254 [2]: #286 (comment)
seanmonstar
approved these changes
Feb 17, 2022
hawkw
added a commit
that referenced
this pull request
Feb 18, 2022
## 0.1.3 (2022-02-18) #### Bug Fixes * record timestamps for updates last (#289) ([703f1aa](703f1aa), closes [#266](266)) * use monotonic `Instant`s for all timestamps (#288) ([abc0830](abc0830), closes [#286](286)) * bail rather than panic when encountering clock skew (#287) ([24db8c6](24db8c6), closes [#286](286)) * fix compilation on targets without 64-bit atomics (#282) ([5590fdb](5590fdb), closes [#279](279))
hawkw
added a commit
that referenced
this pull request
Feb 18, 2022
#### Features * add `Builder::filter_env_var` builder parameter (#276) ([dbdb149](dbdb149), closes [#206](206)) #### Bug Fixes * record timestamps for updates last (#289) ([703f1aa](703f1aa), closes [#266](266)) * use monotonic `Instant`s for all timestamps (#288) ([abc0830](abc0830), closes [#286](286)) * bail rather than panic when encountering clock skew (#287) ([24db8c6](24db8c6), closes [#286](286)) * fix compilation on targets without 64-bit atomics (#282) ([5590fdb](5590fdb), closes [#279](279))
hawkw
added a commit
that referenced
this pull request
Feb 18, 2022
#### Features * add `Builder::filter_env_var` builder parameter (#276) ([dbdb149](dbdb149), closes [#206](206)) #### Bug Fixes * record timestamps for updates last (#289) ([703f1aa](703f1aa), closes [#266](266)) * use monotonic `Instant`s for all timestamps (#288) ([abc0830](abc0830), closes [#286](286)) * bail rather than panic when encountering clock skew (#287) ([24db8c6](24db8c6), closes [#286](286)) * fix compilation on targets without 64-bit atomics (#282) ([5590fdb](5590fdb), closes [#279](279))
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.
Motivation
Currently, if compiled in debug mode, the
console-subscribercrate'sPollStats::end_pollfunction will panic if a poll duration is negative(i.e. if the poll's end
timestamp occurred before its start timestamp). Because the console
currently uses non-monotonic
SystemTimes rather than monotonicInstants (due to serialization requirements), this means we arepotentially succeptible to clock skew adjustments. If one occurs during
a poll, and the system clock goes backwards, we may panic here. This
isn't great!
Solution
This branch fixes the panic by removing the assertions, and changing
end_pollto simply bail and print a warning rather than panicking whenencountering a negative poll duration. This is a short-term solution; a
better long-term solution would be to change
console-subscriberto usemonotonic
Instants rather than non-monotonicSystemTimes (asdiscussed here and here).
Fixes #286