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

Handle dropping wait_for_conn #918

Merged
merged 1 commit into from
Dec 23, 2020
Merged

Handle dropping wait_for_conn #918

merged 1 commit into from
Dec 23, 2020

Conversation

antialize
Copy link
Contributor

If wait_for_conn is dropped before completing, release will call
wake on a waker that no one is listening on. This leads to a state where
waiting will grow indefinitely while all connections are idle.

To fix this we turn waiting into a queue of unique Weak pointers.
This way if wait_for_conn is dropped the pointer in waiting cannot
be upgraded, we can use this as a signal that we shroud wake the
next one instead.

We get into a similar state will happen if wait_for_conn timeouts.

If wait_for_conn is dropped before completing, release will call
wake on a waker that noone is listining on. This leads to a state where
waiting will gro indefinitly while all connections are idle.

To fix this we turn waiting into a queue of unique Weak pointers.
This way if wait_for_conn is dropped the pointer in waiting cannot
be upgraded, we can use this as a signal that we shoud wake the
next one instead.
@abonander
Copy link
Collaborator

We have a pending rewrite of the pool implementation that fixed this by replacing the SegQueue with an intrusive doubly-linked list, but that needs more testing for me to be confident in it.

In the meantime, this is a perfectly good fix that we can put out in a patch release. Thanks.

@abonander abonander merged commit c7cf104 into launchbadge:master Dec 23, 2020
@petersalas
Copy link

Curious if there's a timeline for a patch release with this fix?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants