Deno.Listener#close() doesn't release the address after accept() called

[kt3k]

Deno.Listener#close() can release the listening address synchronously if accept() is not called yet. The below example runs without errors:

let listener = Deno.listen({ port: 3000 });
listener.close();
listener = Deno.listen({ port: 3000 });
listener.close();

However if accept() is called on listener, it can't release the address synchronously at close() call. The below example throws AddrInUse error:

let listener = Deno.listen({ port: 3000 });
const a = listener.accept().then(() => console.log("accepted"), err => console.log(err));
listener.close();
listener = Deno.listen({ port: 3000 });
listener.close();

This behavior blocks port-scanning npm packages (e.g. npm:portfinder, npm:detect-port). ref #25175 #18301

[kt3k]

I tried this patch to force close the tcp socket fd kt3k@31cdde6

But it ended up with the error fatal runtime error: IO Safety violation: owned file descriptor already closed with the 2nd snippet above. I'm not sure there's a way to avoid this error.

[littledivy]

@kt3k Does it work if you move the net_cancel_listener_tcp op after core.close? Like this:

  close() {
    core.close(this.#rid);
    if (this.addr.transport === "tcp") {
      op_net_cancel_listener_tcp(this.#rid);
    }
  }

[littledivy]

Here's an strace for Deno vs Node. https://gist.github.com/littledivy/21d35550829f7dae078c023075af3c70

close() on the first socket is not called by the time it is trying to bind it to a different port. By design, CancelHandle cancellation happens on the next tick. So:

const server = net.createServer(() => {}); // <-- accept().try_or_cancel(&cancel_handle)?.await

server.once("listening", () => {
  server.close(); // <-- cancel_handle.cancel()
  server.listen(8081, host2); // <-- err, cancellation is not complete

  setTimeout(() => server.listen(8081, host2))  // <-- works, cancellation complete on next tick
});

1. For Deno's own TcpListener, I actually think we should preserve this behavior because it works well with async cancellation. Maybe we could update the signature of close to return a Promise?

close() {
  core.close(this.#rid)
  return new Promise(r => setTimout(r, 0));
}

2. For Node.js net API: portfinder is known to be depending on synchronously closing the fd. It should be using the server.close(() => {}) callback. I think your workaround is good enough to keep it working.

[kt3k]

@kt3k Does it work if you move the net_cancel_listener_tcp op after core.close? Like this:

  close() {
    core.close(this.#rid);
    if (this.addr.transport === "tcp") {
      op_net_cancel_listener_tcp(this.#rid);
    }
  }

This caused BadResource: Listener has been closed

Here's an strace for Deno vs Node. https://gist.github.com/littledivy/21d35550829f7dae078c023075af3c70

close() on the first socket is not called by the time it is trying to bind it to a different port. By design, CancelHandle cancellation happens on the next tick. So:

const server = net.createServer(() => {}); // <-- accept().try_or_cancel(&cancel_handle)?.await

server.once("listening", () => {
  server.close(); // <-- cancel_handle.cancel()
  server.listen(8081, host2); // <-- err, cancellation is not complete

  setTimeout(() => server.listen(8081, host2))  // <-- works, cancellation complete on next tick
});

Thanks for the analysis! This is good to know.

1. For Deno's own TcpListener, I actually think we should preserve this behavior because it works well with async cancellation. Maybe we could update the signature of close to return a Promise?

close() {
  core.close(this.#rid)
  return new Promise(r => setTimout(r, 0));
}

This works for Deno.Listener#close, but doesn't for some npm packages as you mention..

2. For Node.js net API: portfinder is known to be depending on synchronously closing the fd. It should be using the server.close(() => {}) callback. I think your workaround is good enough to keep it working.

Divy, thanks for the analysis! I'm closing this for now as there seems no better way than that workaround.