From 4d176b7b7c11aabc584bee45423f108ea47faefe Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Tue, 14 Dec 2021 23:27:04 +0100 Subject: [PATCH] fix(ext/net): make unix and tcp identical on close (#13075) std/http/server knows how to handle "Listener has been closed" exceptions but not "operation canceled" errors. Make "unix" listen sockets throw the same exception as "tcp" listen sockets when the socket is closed and has a pending accept operation. There is still a discrepancy when multiple accept requests are posted but that's probably a less visible issue and something for another day. Fixes #13033 --- cli/tests/unit/net_test.ts | 5 +++-- ext/net/ops.rs | 23 ++++++++++++++--------- ext/net/ops_unix.rs | 7 +++++-- 3 files changed, 22 insertions(+), 13 deletions(-) diff --git a/cli/tests/unit/net_test.ts b/cli/tests/unit/net_test.ts index 7efc7e689d..578820dda8 100644 --- a/cli/tests/unit/net_test.ts +++ b/cli/tests/unit/net_test.ts @@ -147,7 +147,8 @@ Deno.test( listener.close(); await assertRejects( () => p, - Deno.errors.Interrupted, + Deno.errors.BadResource, + "Listener has been closed", ); }, ); @@ -185,7 +186,7 @@ Deno.test( const listener = Deno.listen({ transport: "unix", path: filePath }); let acceptErrCount = 0; const checkErr = (e: Error) => { - if (e instanceof Deno.errors.Interrupted) { // "operation canceled" + if (e.message === "Listener has been closed") { assertEquals(acceptErrCount, 1); } else if (e instanceof Deno.errors.Busy) { // "Listener already in use" acceptErrCount++; diff --git a/ext/net/ops.rs b/ext/net/ops.rs index 739807123e..538eab2d9f 100644 --- a/ext/net/ops.rs +++ b/ext/net/ops.rs @@ -103,6 +103,15 @@ pub(crate) struct AcceptArgs { pub transport: String, } +pub(crate) fn accept_err(e: std::io::Error) -> AnyError { + // FIXME(bartlomieju): compatibility with current JS implementation + if let std::io::ErrorKind::Interrupted = e.kind() { + bad_resource("Listener has been closed") + } else { + e.into() + } +} + async fn accept_tcp( state: Rc>, args: AcceptArgs, @@ -119,15 +128,11 @@ async fn accept_tcp( .try_borrow_mut() .ok_or_else(|| custom_error("Busy", "Another accept task is ongoing"))?; let cancel = RcRef::map(resource, |r| &r.cancel); - let (tcp_stream, _socket_addr) = - listener.accept().try_or_cancel(cancel).await.map_err(|e| { - // FIXME(bartlomieju): compatibility with current JS implementation - if let std::io::ErrorKind::Interrupted = e.kind() { - bad_resource("Listener has been closed") - } else { - e.into() - } - })?; + let (tcp_stream, _socket_addr) = listener + .accept() + .try_or_cancel(cancel) + .await + .map_err(accept_err)?; let local_addr = tcp_stream.local_addr()?; let remote_addr = tcp_stream.peer_addr()?; diff --git a/ext/net/ops_unix.rs b/ext/net/ops_unix.rs index 20d085a5da..58bfa05576 100644 --- a/ext/net/ops_unix.rs +++ b/ext/net/ops_unix.rs @@ -91,8 +91,11 @@ pub(crate) async fn accept_unix( .try_borrow_mut() .ok_or_else(|| custom_error("Busy", "Listener already in use"))?; let cancel = RcRef::map(resource, |r| &r.cancel); - let (unix_stream, _socket_addr) = - listener.accept().try_or_cancel(cancel).await?; + let (unix_stream, _socket_addr) = listener + .accept() + .try_or_cancel(cancel) + .await + .map_err(crate::ops::accept_err)?; let local_addr = unix_stream.local_addr()?; let remote_addr = unix_stream.peer_addr()?;