Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Make duration calculation robust against clock drift #10042

Merged
merged 1 commit into from
Oct 18, 2021

Conversation

bkchr
Copy link
Member

@bkchr bkchr commented Oct 18, 2021

It is possible that Instant::now() is returning an earlier clock time when being called a second
time. To guard against this, we should use saturating_duration_since.

Fixes: paritytech/polkadot#4096

It is possible that `Instant::now()` is returning an earlier clock time when being called a second
time. To guard against this, we should use `saturating_duration_since`.
@bkchr bkchr added A2-insubstantial Pull request requires no code review (e.g., a sub-repository hash update). B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. labels Oct 18, 2021
@arkpar
Copy link
Member

arkpar commented Oct 18, 2021

Documentation clearly states that Instant is monotonic and is not affected by the system clock drift. We rely on that all over the code.

Instants are always guaranteed to be no less than any previously measured instant when created

I think the issue here is that Instant::now is called on two different threads, leading to a race condition. Apparently it is not guaranteed to be monotonic across CPU cores. Here's a relevant stackoverflow discussion: https://stackoverflow.com/questions/46893072/clock-gettimeclock-monotonic-monotonicity-across-cores-threads

@bkchr
Copy link
Member Author

bkchr commented Oct 18, 2021

Documentation clearly states that Instant is monotonic and is not affected by the system clock drift.

Yeah I know. However, afair, we already have seen this in virtual machines as well. That it returned different values per call.

@bkchr
Copy link
Member Author

bkchr commented Oct 18, 2021

bot merge

@paritytech-processbot
Copy link

Bot will approve on the behalf of @bkchr, since they are a team lead, in an attempt to reach the minimum approval count

@paritytech-processbot paritytech-processbot bot merged commit 4a99c09 into master Oct 18, 2021
@paritytech-processbot paritytech-processbot bot deleted the bkchr-fix-client-db-timer branch October 18, 2021 08:18
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A2-insubstantial Pull request requires no code review (e.g., a sub-repository hash update). B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

tokio-runtime-worker panicked at 'supplied instant is later than self'
2 participants