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

Document that you can only listen on an already closed socket #11685

Closed
gibfahn opened this issue Mar 4, 2017 · 3 comments
Closed

Document that you can only listen on an already closed socket #11685

gibfahn opened this issue Mar 4, 2017 · 3 comments
Labels
doc Issues and PRs related to the documentations. good first issue Issues that are suitable for first-time contributors. net Issues and PRs related to the net subsystem.

Comments

@gibfahn
Copy link
Member

gibfahn commented Mar 4, 2017 •
edited
Loading

  • Subsystem: doc

From #8419 (comment),

@addaleax raised a possible concern with #8294 which states in the docs that you can listen() on the same socket multiple times. It should probably say more clearly that you can only listen() on an already closed socket.

EDIT: PR raised: https://github.com/nodejs/node/pull/13149/files
@gibfahn gibfahn added doc Issues and PRs related to the documentations. good first issue Issues that are suitable for first-time contributors. net Issues and PRs related to the net subsystem. labels Mar 4, 2017
@gibfahn gibfahn mentioned this issue Mar 4, 2017
Closed
4 tasks
@joyeecheung
Copy link
Member

Since listen() is overloaded it might help even more to point out what happens if you call listen() on different stuff multiple times..

Also I think listen() doesn't actually raise an error when you try to listen on an unbound handle?

@eduardbme
Copy link
Contributor

will do

@gibfahn
Copy link
Member Author

gibfahn commented Mar 5, 2017

@eduardbcom if you're going to fix this (either in #8419 or a separate PR) then if you could add the Fixes: line to the commit and link to it from here that'd be great!

Fixes: https://github.com/nodejs/node/issues/11685#issuecomment-284221748

@eduardbme eduardbme mentioned this issue May 22, 2017
Closed
4 tasks
eduardbme added a commit to eduardbme/node that referenced this issue Sep 2, 2017
@eduardbme
net: multiple listen() events fail silently
2cd52c9 
Problem:
It's possible to run listen()
  on a net.Server that's already listening to a port.
The result is silent failure,
  with the side effect of changing the connectionKey and or pipeName.

Solution:
  throw an error if listen method called more than once.
  close() method should be called between listen() method calls.

Refs: nodejs#8294
Fixes: nodejs#6190
Fixes: nodejs#11685
addaleax pushed a commit to addaleax/ayo that referenced this issue Sep 5, 2017
@eduardbme @addaleax
net: multiple listen() events fail silently
8d129c3 
Problem:
It's possible to run listen()
  on a net.Server that's already listening to a port.
The result is silent failure,
  with the side effect of changing the connectionKey and or pipeName.

Solution:
  throw an error if listen method called more than once.
  close() method should be called between listen() method calls.

Refs: nodejs/node#8294
Fixes: nodejs/node#6190
Fixes: nodejs/node#11685
PR-URL: nodejs/node#13149
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
doc Issues and PRs related to the documentations. good first issue Issues that are suitable for first-time contributors. net Issues and PRs related to the net subsystem.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

net: multiple listen() events fail silently eduardbme/node
3 participants
@joyeecheung @eduardbme @gibfahn