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.

Sign up for GitHub

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

Jump to bottom

Update the semaphore impl in tower-batch, to avoid hangs #1671

Closed
2 tasks done
teor2345 opened this issue Feb 3, 2021 · 2 comments · Fixed by #1764
Closed
2 tasks done

Update the semaphore impl in tower-batch, to avoid hangs #1671

teor2345 opened this issue Feb 3, 2021 · 2 comments · Fixed by #1764
Labels
A-docs Area: Documentation A-rust Area: Updates to Rust code C-bug Category: This is a bug C-cleanup Category: This is a cleanup
Milestone
2021 Sprint 3

Comments

@teor2345
Copy link
Contributor

teor2345 commented Feb 3, 2021 •
edited
Loading

Is your feature request related to a problem? Please describe.

tower-batch is missing a semaphore fix that wakes waiting clones on close: tower-rs/tower#480

Describe the solution you'd like

Describe alternatives you've considered

Do nothing: tower-buffer users might hang.

Additional context

This issue was discovered during the review in #1593.
@teor2345 teor2345 added A-docs Area: Documentation C-design Category: Software design work A-rust Area: Updates to Rust code C-cleanup Category: This is a cleanup S-needs-triage Status: A bug report needs triage P-Medium labels Feb 3, 2021
@mpguerra mpguerra mentioned this issue Feb 11, 2021
Closed
2 tasks
@teor2345 teor2345 added C-bug Category: This is a bug P-High and removed C-design Category: Software design work P-Medium labels Feb 15, 2021
@teor2345 teor2345 changed the title Review tower-batch::Batch to make sure it matches tower::Buffer Copy the semaphore impl from tower::Buffer 0.3 to tower-batch, to avoid hangs Feb 15, 2021
@teor2345 teor2345 changed the title Copy the semaphore impl from tower::Buffer 0.3 to tower-batch, to avoid hangs Update the semaphore impl in tower-batch, to avoid hangs Feb 15, 2021
@teor2345 teor2345 modified the milestones: 2021 Sprint 3, 2021 Sprint 4 Feb 15, 2021
@teor2345
Copy link
Contributor Author

teor2345 commented Feb 15, 2021 •
edited
Loading

This issue can actually cause hangs, so I've added it to the next sprint.
(But we're not sure how often they happen in practice, because we haven't used tower-buffer very much yet.)

This was referenced Feb 15, 2021
Closed
Merged
@mpguerra mpguerra removed the S-needs-triage Status: A bug report needs triage label Feb 18, 2021
@mpguerra mpguerra modified the milestones: 2021 Sprint 4, 2021 Sprint 3 Feb 22, 2021
@teor2345
Copy link
Contributor Author

This fix was incomplete, it also needs some changes in tower_buffer::Buffer (and maybe other files).

We should follow tower-rs/tower#480 almost exactly.

@teor2345 teor2345 reopened this Feb 23, 2021
@teor2345 teor2345 mentioned this issue Feb 23, 2021
Closed
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-docs Area: Documentation A-rust Area: Updates to Rust code C-bug Category: This is a bug C-cleanup Category: This is a cleanup
Projects
None yet
Milestone
2021 Sprint 3
Development

Successfully merging a pull request may close this issue.

Incomplete: Wake waiting tower-batch clones on close teor2345/zebra
2 participants
@mpguerra @teor2345