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.

Sign up for GitHub

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

Jump to bottom

ws removes underlying socket's "internal" event listeners #392

Closed
misterdjules opened this issue Oct 31, 2014 · 5 comments
Closed

ws removes underlying socket's "internal" event listeners #392

misterdjules opened this issue Oct 31, 2014 · 5 comments

Comments

@misterdjules
Copy link

This leads to the issue described in this comment: nodejs/node-v0.x-archive#8615 (comment).

To paraphrase the comment mentioned above, when the end event is emitted on the underlying socket, this._socket.removeAllListeners removes all custom events listeners setup by ws, but it also removes "internal" event listeners that need to be called to properly close it. This results in the other end of the connection waiting for it to be properly closed.
misterdjules pushed a commit to misterdjules/ws that referenced this issue Oct 31, 2014
Do not remove sockets' internal event listeners
4b15588 
When adding a "custom" event listener to the underlyin socket, use the
functions named "addCustomEventListener" or "addCustomEventListeners".
This will keep track of which event listeners have been added. When it's
time to remove custom events listeners, use
removeAllCustomEventListeners.

Fixes websockets#392
misterdjules pushed a commit to misterdjules/ws that referenced this issue Oct 31, 2014
Do not remove sockets' internal event listeners
972bf6e 
When adding a "custom" event listener to the underlyin socket, use the
functions named "addCustomEventListener" or "addCustomEventListeners".
This will keep track of which event listeners have been added. When it's
time to remove custom events listeners, use
removeAllCustomEventListeners.

Fixes websockets#392
This was referenced Oct 31, 2014
Closed
Closed
@migounette
Copy link

@3rd-Eden Any comment on the fix ?
On my side we are waiting for it, whith the out of node 0.12.0 for next week.
Thanks for your lights.

@migounette
Copy link

@3rd-Eden So far so good, short run is perfect. Waiting the result of 12 hours of run.
Thanks

@3rd-Eden
Copy link
Member

Published as 0.5

@migounette
Copy link

Fix Confirmed.

No more memory leak or FD leak after 10 hours of run with high traffic

isfmemory
isffd

@3rd-Eden
Copy link
Member

Great!

@calzoneman calzoneman mentioned this issue Mar 2, 2015
Closed
@lpinca lpinca mentioned this issue Nov 20, 2016
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Do not remove sockets' internal event listeners misterdjules/ws
3 participants
@3rd-Eden @misterdjules @migounette