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

Avoid leaking watch threads on client shutdown #87

Merged

Conversation

behos
Copy link
Contributor

@behos behos commented Oct 14, 2022

When creating the ZkWatch struct, we keep a reference to the sender of the channel and clone it on sender() to pass along to the ZkIo struct.

When the ZkIo thread finishes, its copy of the sender is dropped but the channel is not closed because the watch has a reference to its own sender, therefore leaking the thread.

Instead, we can prevent the watch from ever taking the sender by returning it through the constructor, so that it can be owned by ZkIo which will close the channel as expected on shutdown.

When creating the ZkWatch struct, we keep a reference to the sender of
the channel and clone it on `sender()` to pass along to the `ZkIo`
struct.

When the ZkIo thread finishes, its copy of the sender is dropped but the
channel is not closed because the watch has a reference to its own
sender, therefore leaking the thread.

Instead, we can prevent the watch from ever taking the sender by
returning it through the constructor, so that it can be owned by `ZkIo`
which will close the channel as expected on shutdown.
@bonifaido bonifaido self-assigned this Oct 17, 2022
Copy link
Owner

@bonifaido bonifaido left a comment

Choose a reason for hiding this comment

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

LGTM, this breaks the API a bit, so I will bump a version number here.

Thank you!

@bonifaido bonifaido merged commit 5bd4cbe into bonifaido:master Oct 17, 2022
@behos behos deleted the giorgos.georgiou/close-leaky-threads branch October 17, 2022 11:18
@behos behos restored the giorgos.georgiou/close-leaky-threads branch October 17, 2022 11:18
@behos behos deleted the giorgos.georgiou/close-leaky-threads branch October 17, 2022 11:22
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.

2 participants