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

Wrapping existing net.Socket does not work #37873

Open
marco-a opened this issue Mar 23, 2021 · 8 comments
Open

Wrapping existing net.Socket does not work #37873

marco-a opened this issue Mar 23, 2021 · 8 comments
Labels
doc Issues and PRs related to the documentations. net Issues and PRs related to the net subsystem.

Comments

@marco-a
Copy link

marco-a commented Mar 23, 2021

  • Version: 14.16.0
  • Platform: MacOS
  • Subsystem: macOS Catalina 10.15.7

What steps will reproduce the bug?

const net = require("net")

const srv = net.createServer()

srv.on("connection", c => {
    const sock = new net.Socket({
        fd: c._handle.fd
    })
})

srv.listen(4444)

How often does it reproduce? Is there a required condition?

No required condition (as far as I can tell).

What is the expected behavior?

That a new net.Socket instance is created which behaves exactly like the original instance (c in this case).

What do you see instead?

An error is thrown (EEXISTS):

net.js:337
        throw errnoException(err, 'open');
        ^

Error: open EEXIST
    at new Socket (net.js:337:15)
    at Server.<anonymous> (test.js:6:15)
    at Server.emit (events.js:315:20)
    at TCP.onconnection (net.js:1560:8) {
  errno: -17,
  code: 'EEXIST',
  syscall: 'open'
}

Additional information

@Ayase-252
Copy link
Member

I don't think you need to wrap with net.Socket again, since c is already an opened net.Socket.

https://nodejs.org/api/net.html#net_event_connection

@marco-a
Copy link
Author

marco-a commented Mar 23, 2021

I'm aware that c is already a net.Socket.

@Ayase-252
Copy link
Member

Ayase-252 commented Mar 23, 2021

@marco-a

Hi, Sorry. I'm just aware that the expected behavior section is missing. Could you provide what you are expecting from the snippet?

@marco-a
Copy link
Author

marco-a commented Mar 23, 2021

Sure.

What I was expecting is that the net.Socket instance is created and that it behaves exactly like the original instance (c in this case).

@marco-a
Copy link
Author

marco-a commented Mar 23, 2021

I will adapt my code to use c directly, but I still think this is a bug or the documentation should be updated.

fd If specified, wrap around an existing socket with the given file descriptor, otherwise a new socket will be created.

In my view net.Socket (or c) is an existing socket, also I'm not aware of any other socket type than net.Socket in node (but I could be wrong).

@Ayase-252
Copy link
Member

Sure, I agree with you that the part of document needs improve if it leads misuse.

Would you interest in opening a PR to correct that?

@Ayase-252 Ayase-252 added doc Issues and PRs related to the documentations. net Issues and PRs related to the net subsystem. labels Mar 23, 2021
@Ayase-252
Copy link
Member

Ayase-252 commented Mar 23, 2021

I also need help to understand the exact meaning of words in document.

fd If specified, wrap around an existing socket with the given file descriptor, otherwise a new socket will be created.

Since wrapping an existing socket seems throwing error here, what type of existing socket can be passed here?

cc @nodejs/net

@theanarkh
Copy link
Contributor

theanarkh commented Jun 15, 2022

Error is thrown from Libuv, code is as follow.

int uv__fd_exists(uv_loop_t* loop, int fd) {
  return (unsigned) fd < loop->nwatchers && loop->watchers[fd] != NULL;
}

related:
#3604
libuv/libuv#1851

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

No branches or pull requests

3 participants