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

UnreadyService ignores dropped cancel handles #3130

Closed
jvff opened this issue Dec 2, 2021 · 1 comment · Fixed by #3200
Closed

UnreadyService ignores dropped cancel handles #3130

jvff opened this issue Dec 2, 2021 · 1 comment · Fixed by #3200
Assignees
Labels
A-network Area: Network protocol updates or fixes C-bug Category: This is a bug C-enhancement Category: This is an improvement I-hang A Zebra component stops responding to requests I-heavy Problems with excessive memory, disk, or CPU usage I-slow Problems with performance or responsiveness

Comments

@jvff
Copy link
Contributor

jvff commented Dec 2, 2021

Motivation

The UnreadyService allows the request in flight to be cancelled through a oneshot channel. However, if the cancel handle (the oneshot::Sender endpoint) is dropped, the request isn't cancelled. This can lead to bugs and requests being kept executing when they should have been cancelled.

Alternatives

  1. It might make sense to replace the "cancel handle" with some sort of LiveGuard type, that automatically signals that the request should be cancelled in its Drop implementation.

  2. We could also check for Err(Canceled) in the existing code:

    if let Poll::Ready(Ok(CancelClientWork)) = this.cancel.poll(cx) {

Related Work

Teor uncovered this issue while reviewing #3108.

@jvff jvff added C-enhancement Category: This is an improvement S-needs-triage Status: A bug report needs triage labels Dec 2, 2021
@teor2345 teor2345 added A-network Area: Network protocol updates or fixes C-bug Category: This is a bug I-hang A Zebra component stops responding to requests I-heavy Problems with excessive memory, disk, or CPU usage I-slow Problems with performance or responsiveness P-Medium labels Dec 2, 2021
@mpguerra
Copy link
Contributor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-network Area: Network protocol updates or fixes C-bug Category: This is a bug C-enhancement Category: This is an improvement I-hang A Zebra component stops responding to requests I-heavy Problems with excessive memory, disk, or CPU usage I-slow Problems with performance or responsiveness
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants