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

The WebSocket client pool should not starve on connect error. #5440

Merged
merged 1 commit into from
Jan 10, 2025

Conversation

vietj
Copy link
Member

@vietj vietj commented Jan 10, 2025

Motivation:

The client WebSocket pool implementation (which is not really a pool but instead a semaphore + a wait queue) does not decrement the semaphore when the connection fails to be established.

Currently, no operation is done beside calling back the caller. As consequence, the wait queue fills with new requests and never satisfy them since.

Changes:

When the connection fails, the semaphore should be released and check the wait queue for pending requests.

@vietj vietj added the bug label Jan 10, 2025
@vietj vietj self-assigned this Jan 10, 2025
Motivation:

The client WebSocket pool implementation (which is not really a pool but instead a semaphore + a wait queue) does not decrement the semaphore when the connection fails to be established.

Currently, no operation is done beside calling back the caller. As consequence, the wait queue fills with new requests and never satisfy them since.

Changes:

When the connection fails, the semaphore should be released and check the wait queue for pending requests.
@vietj vietj force-pushed the websocket-pool-should-not-starve-on-connect-error branch from 41472e0 to 247baad Compare January 10, 2025 09:00
@vietj vietj merged commit 77c2b91 into master Jan 10, 2025
8 checks passed
@vietj vietj added this to the 5.0.0 milestone Jan 10, 2025
@vietj vietj deleted the websocket-pool-should-not-starve-on-connect-error branch January 10, 2025 09:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant