New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Eliminate Receiver::recv_timeout panic #56827

Merged
merged 2 commits into from Jan 2, 2019

Conversation

Projects
None yet
4 participants
@faern
Copy link
Contributor

faern commented Dec 14, 2018

Fixes #54552.

This panic is because recv_timeout uses Instant::now() + timeout internally. This possible panic is not mentioned in the documentation for this method.

Very recently we merged (still unstable) support for checked addition (#56490) of Instant + Duration, so it's now finally possible to add these together without risking a panic.

@rust-highfive

This comment has been minimized.

Copy link
Collaborator

rust-highfive commented Dec 14, 2018

r? @KodrAus

(rust_highfive has picked a reviewer for you, use r? to override)

@KodrAus

This comment has been minimized.

Copy link

KodrAus commented Jan 1, 2019

Thanks @faern! The documentation updates are appreciated 👍

The assumption that the only reason this would overflow is because a massive duration has been supplied seems ok to me, but I apologize in advance to any time travelers suffering from Y300000K bugs and looking for someone to blame.

@bors r+

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Jan 1, 2019

📌 Commit 496f547 has been approved by KodrAus

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Jan 2, 2019

⌛️ Testing commit 496f547 with merge 9653034...

bors added a commit that referenced this pull request Jan 2, 2019

Auto merge of #56827 - faern:eliminate-recv-timeout-panic, r=KodrAus
Eliminate Receiver::recv_timeout panic

Fixes #54552.

This panic is because `recv_timeout` uses `Instant::now() + timeout` internally. This possible panic is not mentioned in the documentation for this method.

Very recently we merged (still unstable) support for checked addition (#56490) of `Instant + Duration`, so it's now finally possible to add these together without risking a panic.
@bors

This comment has been minimized.

Copy link
Contributor

bors commented Jan 2, 2019

☀️ Test successful - status-appveyor, status-travis
Approved by: KodrAus
Pushing 9653034 to master...

@bors bors merged commit 496f547 into rust-lang:master Jan 2, 2019

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment