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

net: pending socket re-use #30832

Open
ronag opened this issue Dec 7, 2019 · 4 comments
Open

net: pending socket re-use #30832

ronag opened this issue Dec 7, 2019 · 4 comments
Labels
net Issues and PRs related to the net subsystem.

Comments

@ronag
Copy link
Member

ronag commented Dec 7, 2019

It looks like net.Socket instances are intended to be re-used however while reading through the code I spot some potential problems:

  • _destroy(): doesn't wait for connect to finish
  • connect: doesn't first destroy if already connected, before connecting again
  • connect: doesn't wait for pending destroy to finish, before calling e.g. _undestroy()

Not sure if these are actual problems that need to be fixed?

@mcollina

@ronag ronag changed the title net socket re-use net: pending socket re-use Dec 7, 2019
@mcollina
Copy link
Member

mcollina commented Dec 7, 2019

I think those things would ideally need to be fixed, however this is a really obscure feature of Node.js, and I suspect most do not even know that is there or why it's needed. If the fix are trivial, go for them. If they are complex, then it's probably not worth it.

@ronag
Copy link
Member Author

ronag commented Dec 7, 2019

I'll look into it when porting it over to _construct.

If they are complex, then it's probably not worth it.

Would it be an option to hard deprecate re-use and error when trying to do so?

@mcollina
Copy link
Member

mcollina commented Dec 7, 2019

Would it be an option to hard deprecate re-use and error when trying to do so?

Possibly, I'm not sure if there are some usage in the wild.

@BridgeAR BridgeAR added the net Issues and PRs related to the net subsystem. label Dec 20, 2019
@ronag
Copy link
Member Author

ronag commented Mar 10, 2020

I find connect w/ re-use kind of dangerous. Could we doc deprecate, add a runtime warning or something along those lines?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
net Issues and PRs related to the net subsystem.
Projects
None yet
Development

No branches or pull requests

3 participants