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

Official socket transfer support #8974

Closed
ghost opened this issue Oct 7, 2016 · 10 comments
Closed

Official socket transfer support #8974

ghost opened this issue Oct 7, 2016 · 10 comments
Labels
feature request Issues that request new features to be added to Node.js. net Issues and PRs related to the net subsystem.

Comments

@ghost
Copy link

ghost commented Oct 7, 2016

I'm interested in an official way of transferring a socket from Node.js (net.Socket / tls.TLSSocket) representation to the native representation (file descriptor / SOCKET descriptor + SSL pointer).

This can be used to transfer a connection from JS land into an addon, and have the addon manage the connection way more efficient.

@Fishrock123 Fishrock123 added the question Issues that look for answers. label Oct 7, 2016
@Fishrock123
Copy link
Contributor

Fishrock123 commented Oct 7, 2016

Once you call .listen() you can access the net.Socket._handle property, which in turn has a fd property.

_handle and _handle.fd are unlikely to be removed, and definitely will not disappear without significant fanfare with prior deprecation and semver-major (still, very unlikely). You should be good to use it, even if it is undocumented.

@ghost
Copy link
Author

ghost commented Oct 7, 2016

Windows version has no .fd of its _handle.

@ghost
Copy link
Author

ghost commented Oct 7, 2016

#7627

@mscdex mscdex added the net Issues and PRs related to the net subsystem. label Oct 7, 2016
@Fishrock123 Fishrock123 added feature request Issues that request new features to be added to Node.js. and removed question Issues that look for answers. labels Oct 7, 2016
@ghost
Copy link
Author

ghost commented Oct 8, 2016

I think the first step in solving this is to agree on an interface. When that is done, the implementation should be very simple. I know @bnoordhuis is against the idea of simply exposing the native SOCKET handle on Windows and he seems to prefer having a strictly transferring-only interface so that addons do not mess around with the internals of any still existing Node.js object.

@ghost
Copy link
Author

ghost commented Oct 8, 2016

Any kind of net.Socket.export((fd, ssl) => {
// Here we can do stuff to the fd and ssl pointers. Fd can be int or SOCKET
});

works perfectly for me

@Trott
Copy link
Member

Trott commented Jul 12, 2017

This can be used to transfer a connection from JS land into an addon,

@bnoordhuis @indutny @nodejs/streams @nodejs/n-api Thoughts on how to do it now, how a new feature might do it, whether it is something that should be done at all, whether or not this issue should remain open?

@mcollina
Copy link
Member

I think the best way to do this is via uv_link_t, if that would ever be supported here. This is an issue also on core itsef, as I would love to not pass through JS to read data from TLS.

https://github.com/indutny/uv_link_t

I do not think that manipulating directly the fd and pointers is the way to go. Linking to the OpenSSL version that node use is impossible (I could not do it, maybe someone else can), so allowing this is not a good idea.

@bnoordhuis
Copy link
Member

bnoordhuis commented Jul 12, 2017

whether it is something that should be done at all

It's a pretty niche thing. Not saying no but there would probably only be a few users at best, making it not worthwhile to sink a lot of time into.

There's also the question of if you want to manage sockets yourself, why not create them yourself?

Linking to the OpenSSL version that node use is impossible

This has improved in the last year, see #6274.

@mcollina
Copy link
Member

@bnoordhuis thanks!

@bnoordhuis
Copy link
Member

It's been a year since this issue was filed and there doesn't seem to be a whole lot of interest, never mind the practical considerations.

#7627 was closed as well so I'll close this out too. Please reopen if you think this needs to be revisited.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Issues that request new features to be added to Node.js. net Issues and PRs related to the net subsystem.
Projects
None yet
Development

No branches or pull requests

5 participants