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

security: avoid hangs in the peer set and related code #7858

Closed
teor2345 opened this issue Oct 27, 2023 · 3 comments · Fixed by #7859
Closed

security: avoid hangs in the peer set and related code #7858

teor2345 opened this issue Oct 27, 2023 · 3 comments · Fixed by #7859
Assignees
Labels
A-network Area: Network protocol updates or fixes C-bug Category: This is a bug C-tech-debt Category: Code maintainability issues I-hang A Zebra component stops responding to requests I-remote-trigger Remote nodes can make Zebra do something bad S-needs-triage Status: A bug report needs triage

Comments

@teor2345
Copy link
Contributor

teor2345 commented Oct 27, 2023

Motivation

There are still some potential hangs in the peer set and related code.

Specifications

Rust futures need to give a copy of their task waker to any code that could potentially wake the task.

Complex Code or Requirements

Method calls shouldn't be skipped based on the outcomes of previous methods, unless the connection is shutting down.

Testing

This is fairly straightforward, we need to check that each method that could wake the service or future is called.

We have existing tests that cover this code.

Related Work

Follow up to #7772.

@teor2345 teor2345 added C-bug Category: This is a bug S-needs-triage Status: A bug report needs triage P-Low ❄️ I-hang A Zebra component stops responding to requests I-usability Zebra is hard to understand or use A-network Area: Network protocol updates or fixes labels Oct 27, 2023
@mpguerra mpguerra added this to Zebra Oct 27, 2023
@github-project-automation github-project-automation bot moved this to 🆕 New in Zebra Oct 27, 2023
@teor2345 teor2345 changed the title security: when the peer set is empty, wake waiting tasks for new peers or ready connected peers security: when the peer set is empty, wake waiting tasks for the first new peer or ready connected peer Oct 27, 2023
@teor2345 teor2345 added I-remote-trigger Remote nodes can make Zebra do something bad and removed I-usability Zebra is hard to understand or use labels Oct 30, 2023
@arya2 arya2 added the C-tech-debt Category: Code maintainability issues label Oct 30, 2023
@teor2345 teor2345 changed the title security: when the peer set is empty, wake waiting tasks for the first new peer or ready connected peer security: avoid hangs in the peer set and related code Nov 6, 2023
@mpguerra
Copy link
Contributor

mpguerra commented Nov 7, 2023

Hey team! Please add your planning poker estimate with Zenhub @arya2 @oxarbitrage @teor2345 @upbqdn

@teor2345 teor2345 self-assigned this Nov 7, 2023
@mpguerra
Copy link
Contributor

@teor2345 Do you remember why we decided to move this to next Sprint (Sprint 24)? I see that we have PR #7859 open for this issue already which is why I've moved it back to this sprint (Sprint 23).

@teor2345
Copy link
Contributor Author

@teor2345 Do you remember why we decided to move this to next Sprint (Sprint 24)? I see that we have PR #7859 open for this issue already which is why I've moved it back to this sprint (Sprint 23).

It was a lower priority, and we thought other tickets would be ready earlier. (But it turned out they weren't.)

So it's fine to do this now, it's unlikely to extend into next sprint unless the review is both late and large.

@mergify mergify bot closed this as completed in #7859 Nov 16, 2023
@github-project-automation github-project-automation bot moved this from 🆕 New to ✅ Done in Zebra Nov 16, 2023
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-tech-debt Category: Code maintainability issues I-hang A Zebra component stops responding to requests I-remote-trigger Remote nodes can make Zebra do something bad S-needs-triage Status: A bug report needs triage
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants