Skip to content
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

Add pointers about idle timeout in documentation #1875

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions quiche/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5850,6 +5850,15 @@ impl Connection {
/// Processes a timeout event.
///
/// If no timeout has occurred it does nothing.
///
/// Note that while this method handles the _detection_ of timeouts, it
/// does not prevent them. When a `max_idle_timeout` is configured in the
/// transport parameters, the application must make sure to not be idle for
/// that long, either by sending data, or by calling [`send_ack_eliciting`]
/// if there is no data to send. See also the [Deferring Idle Timeout
/// section of RFC 9000][idle].
///
/// [idle]: https://www.rfc-editor.org/rfc/rfc9000.html#name-deferring-idle-timeout
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding a note to help users seems sensible but I don't think the proposal covers the considerations with enough nuance.

The QUIC idle timeout is negotiated between endpoints, so it doesn't necessarily matter what the local endpoint configured, since the peer can affect it. This suggests that we might want to expose the value of the idle timeout. timeout() and timeout_instant() can return values unrelated to the idle timeout, so aren't necessarily correct.

Also, idle timeout is a useful feature of QUIC, so we shouldn't unilaterally recommend that applications keep a connection alive when there may be no good reason to. The RFC text seems sufficient in terms of guidance. Pointing to the functions that reset the idle timer seems helpful though.

pub fn on_timeout(&mut self) {
let now = time::Instant::now();

Expand Down