Skip to content

Conversation

@dpsarrou
Copy link

Related to Issue #5

Problem

The current mechanism of pushing the WebSocket responses to subscription channels is prone to blocking.
For example, if one subscribes to the Ticker for multiple markets but does not consume all the ticker events from all channels (eg. at some point you are no longer interested in a specific ticker and you stop reading from its channel) then the whole handleMessage function will block at the point where the specific channel has reached maximum capacity (100) and cannot push any more messages. This will prevent further consumption of WebSocket responses as well.

Solution

This PR addresses the above with the bare minimum effort. I have restrained from doing any further code improvements (including modularization, formatting, or addressing issues in other parts) in hope that this can be a small change that can be reviewed and accepted quickly. The approach is simple:

  • Move the event handling code into its own method for easier refactoring
  • Convert the parts where a message is sent to the subscription channel into a select{} with a default case so the operation becomes non-blocking
  • Add basic Unsubscribe methods for every Subscription method (supported by the API but were not implemented in this client)

What this PR does not address

  • Draining the channels that have been unsubscribed
  • Possible block in the addTooBook function

@dpsarrou dpsarrou force-pushed the non_blocking_subscription_channels branch from be5e7b1 to d524672 Compare November 11, 2022 09:49
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

No Coverage information No Coverage information
0.0% 0.0% Duplication

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.

1 participant