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

fix(dht): WebsocketServer memory leak #2080

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

teogeb
Copy link
Contributor

@teogeb teogeb commented Nov 20, 2023

Improve WebsocketServer cleanup. When we explicitly remove httpServer listeners, the instance doesn't leak memory immediately after WebsocketServer#stop is called.

There wasn't a permanent memory leak: e.g. await wait(100) before the check in the unit test would also make the test pass.

Open questions

Why the change causes this tests to fail:

dht-browser:

1) starts and stops
     WebsocketServer
     ReferenceError: require is not defined

@github-actions github-actions bot added the dht Related to DHT package label Nov 20, 2023
@teogeb teogeb requested a review from juslesan November 20, 2023 10:31
@teogeb teogeb requested a review from ptesavol December 1, 2023 13:32
Copy link
Collaborator

@ptesavol ptesavol left a comment

Choose a reason for hiding this comment

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

Agreed.

Base automatically changed from streamr-1.0 to main December 27, 2023 08:31
Copy link
Contributor

@juslesan juslesan left a comment

Choose a reason for hiding this comment

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

What is the status of this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dht Related to DHT package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants