Skip to content
This repository has been archived by the owner on Oct 19, 2024. It is now read-only.

fix: drop WS server if work complete #399

Merged
merged 6 commits into from
Aug 22, 2021

Conversation

mattsse
Copy link
Collaborator

@mattsse mattsse commented Aug 20, 2021

Motivation

This fixes another edge case that leads to active WS servers with not possibility of future work.
If all instruction senders are dropped there are no more subscriptions or pending requests, then this leaves the Ws server with nothing todo and since the receiver can't receive any more instructions it's effectively useless, running in the background with, still connected to the ws server and occasionally receiving/sending pings/pongs.
Also Subscriptions with a closed channel also remained active.

Solution

  • Remove a subscription from the subscriptions if its channel was closed
  • break the tick loop so the server gets dropped if the instructions receiver channel is closed and no more subscriptions are active or requests are pending.

@mattsse mattsse mentioned this pull request Aug 22, 2021
3 tasks
@mattsse
Copy link
Collaborator Author

mattsse commented Aug 22, 2021

Update

added an is_done check in the tick loop instead of checking this abort condition in the tick function

Copy link
Owner

@gakonst gakonst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - nice work.

@gakonst gakonst merged commit bfbbee5 into gakonst:master Aug 22, 2021
meetmangukiya pushed a commit to meetmangukiya/ethers-rs that referenced this pull request Mar 21, 2022
* Pull latest ethers

* Do not suppress warnings on build

* Add `--ignored-error-codes` option

* chore: remove double success with warnings log

* chore: fix dai resolve addr test

https://github.com/gakonst/ethers-rs/pull/771/files

* fix: do not log output warnings if ignored

using gakonst#775,
warnings for test contracts are not going to be logged.

Co-authored-by: Georgios Konstantopoulos <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants