Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

refactor stratum to remove retain cycle #7827

Merged
merged 3 commits into from
Feb 7, 2018
Merged

refactor stratum to remove retain cycle #7827

merged 3 commits into from
Feb 7, 2018

Conversation

debris
Copy link
Collaborator

@debris debris commented Feb 7, 2018

fixed #7823

@debris debris added A0-pleasereview 🤓 Pull request needs code review. M4-core ⛓ Core client code / Rust. labels Feb 7, 2018
@debris debris requested a review from tomusdrw February 7, 2018 11:36
Copy link
Collaborator

@tomusdrw tomusdrw left a comment

Choose a reason for hiding this comment

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

Looks good.

Can be further simplified by using PubSub from jsonrpc instead of manually tracking peers.

In such case TCP dispatcher does not need to be exposed from the builder at all. You just use RequestContext::sender to have a reliable sink to the peer and also pub-sub handles disconnects/connection drops automatically (by calling unsubscribe handler).

@tomusdrw tomusdrw added A7-looksgoodtestsfail 🤖 Pull request is reviewed well, but cannot be merged due to tests failing. and removed A0-pleasereview 🤓 Pull request needs code review. labels Feb 7, 2018
@debris debris added A8-looksgood 🦄 Pull request is reviewed well. and removed A7-looksgoodtestsfail 🤖 Pull request is reviewed well, but cannot be merged due to tests failing. labels Feb 7, 2018
@debris debris merged commit f244ebe into master Feb 7, 2018
@debris debris deleted the stratum_refactor branch February 7, 2018 16:15
@5chdn 5chdn added this to the 1.10 milestone Feb 8, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well. M4-core ⛓ Core client code / Rust.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Retain cycle in stratum
3 participants