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

fix(ext/net): don't remove sockets on unix listen #16394

Merged
merged 1 commit into from
Oct 23, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions cli/tests/unit/http_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
delay,
fail,
} from "./test_util.ts";
import { join } from "../../../test_util/std/path/mod.ts";

async function writeRequestAndReadResponse(conn: Deno.Conn): Promise<string> {
const encoder = new TextEncoder();
Expand Down Expand Up @@ -1290,14 +1291,19 @@ Deno.test(
},
);

function tmpUnixSocketPath(): string {
const folder = Deno.makeTempDirSync();
return join(folder, "socket");
}

// https://github.com/denoland/deno/pull/13628
Deno.test(
{
ignore: Deno.build.os === "windows",
permissions: { read: true, write: true },
},
async function httpServerOnUnixSocket() {
const filePath = Deno.makeTempFileSync();
const filePath = tmpUnixSocketPath();

let httpConn: Deno.HttpConn;
const promise = (async () => {
Expand Down Expand Up @@ -2239,7 +2245,7 @@ Deno.test("upgradeHttp unix", {
permissions: { read: true, write: true },
ignore: Deno.build.os === "windows",
}, async () => {
const filePath = Deno.makeTempFileSync();
const filePath = tmpUnixSocketPath();
const promise = deferred();

async function client() {
Expand Down Expand Up @@ -2435,7 +2441,7 @@ Deno.test(
permissions: { read: true, write: true },
},
async function httpServerWithoutExclusiveAccessToUnixSocket() {
const filePath = await Deno.makeTempFile();
const filePath = tmpUnixSocketPath();
const listener = Deno.listen({ path: filePath, transport: "unix" });

const [clientConn, serverConn] = await Promise.all([
Expand Down
56 changes: 39 additions & 17 deletions cli/tests/unit/net_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
delay,
execCode,
} from "./test_util.ts";
import { join } from "../../../test_util/std/path/mod.ts";

let isCI: boolean;
try {
Expand Down Expand Up @@ -43,13 +44,18 @@ Deno.test(
},
);

function tmpUnixSocketPath(): string {
const folder = Deno.makeTempDirSync();
return join(folder, "socket");
cjihrig marked this conversation as resolved.
Show resolved Hide resolved
}

Deno.test(
{
ignore: Deno.build.os === "windows",
permissions: { read: true, write: true },
},
function netUnixListenClose() {
const filePath = Deno.makeTempFileSync();
const filePath = tmpUnixSocketPath();
const socket = Deno.listen({
path: filePath,
transport: "unix",
Expand All @@ -66,7 +72,7 @@ Deno.test(
permissions: { read: true, write: true },
},
function netUnixPacketListenClose() {
const filePath = Deno.makeTempFileSync();
const filePath = tmpUnixSocketPath();
const socket = Deno.listenDatagram({
path: filePath,
transport: "unixpacket",
Expand All @@ -84,7 +90,7 @@ Deno.test(
},
function netUnixListenWritePermission() {
assertThrows(() => {
const filePath = Deno.makeTempFileSync();
const filePath = tmpUnixSocketPath();
const socket = Deno.listen({
path: filePath,
transport: "unix",
Expand All @@ -103,7 +109,7 @@ Deno.test(
},
function netUnixPacketListenWritePermission() {
assertThrows(() => {
const filePath = Deno.makeTempFileSync();
const filePath = tmpUnixSocketPath();
const socket = Deno.listenDatagram({
path: filePath,
transport: "unixpacket",
Expand Down Expand Up @@ -139,7 +145,7 @@ Deno.test(
permissions: { read: true, write: true },
},
async function netUnixCloseWhileAccept() {
const filePath = await Deno.makeTempFile();
const filePath = tmpUnixSocketPath();
const listener = Deno.listen({
path: filePath,
transport: "unix",
Expand Down Expand Up @@ -183,7 +189,7 @@ Deno.test(
permissions: { read: true, write: true },
},
async function netUnixConcurrentAccept() {
const filePath = await Deno.makeTempFile();
const filePath = tmpUnixSocketPath();
const listener = Deno.listen({ transport: "unix", path: filePath });
let acceptErrCount = 0;
const checkErr = (e: Error) => {
Expand Down Expand Up @@ -317,7 +323,7 @@ Deno.test(
permissions: { read: true, write: true },
},
async function netUnixDialListen() {
const filePath = await Deno.makeTempFile();
const filePath = tmpUnixSocketPath();
const listener = Deno.listen({ path: filePath, transport: "unix" });
listener.accept().then(
async (conn) => {
Expand Down Expand Up @@ -463,28 +469,29 @@ Deno.test(
permissions: { read: true, write: true },
},
async function netUnixPacketSendReceive() {
const filePath = await Deno.makeTempFile();
const aliceFilePath = tmpUnixSocketPath();
const alice = Deno.listenDatagram({
path: filePath,
path: aliceFilePath,
transport: "unixpacket",
});
assert(alice.addr.transport === "unixpacket");
assertEquals(alice.addr.path, filePath);
assertEquals(alice.addr.path, aliceFilePath);

const bobFilePath = tmpUnixSocketPath();
const bob = Deno.listenDatagram({
path: filePath,
path: bobFilePath,
transport: "unixpacket",
});
assert(bob.addr.transport === "unixpacket");
assertEquals(bob.addr.path, filePath);
assertEquals(bob.addr.path, bobFilePath);

const sent = new Uint8Array([1, 2, 3]);
const byteLength = await alice.send(sent, bob.addr);
assertEquals(byteLength, 3);

const [recvd, remote] = await bob.receive();
assert(remote.transport === "unixpacket");
assertEquals(remote.path, filePath);
assertEquals(remote.path, aliceFilePath);
assertEquals(recvd.length, 3);
assertEquals(1, recvd[0]);
assertEquals(2, recvd[1]);
Expand All @@ -494,11 +501,11 @@ Deno.test(
},
);

// TODO(piscisaureus): Enable after Tokio v0.3/v1.0 upgrade.
// TODO(lucacasonato): support concurrent reads and writes on unixpacket sockets
Deno.test(
{ ignore: true, permissions: { read: true, write: true } },
async function netUnixPacketConcurrentSendReceive() {
const filePath = await Deno.makeTempFile();
const filePath = tmpUnixSocketPath();
const socket = Deno.listenDatagram({
path: filePath,
transport: "unixpacket",
Expand Down Expand Up @@ -584,7 +591,7 @@ Deno.test(
permissions: { read: true, write: true },
},
async function netUnixListenCloseWhileIterating() {
const filePath = Deno.makeTempFileSync();
const filePath = tmpUnixSocketPath();
const socket = Deno.listen({ path: filePath, transport: "unix" });
const nextWhileClosing = socket[Symbol.asyncIterator]().next();
socket.close();
Expand All @@ -601,7 +608,7 @@ Deno.test(
permissions: { read: true, write: true },
},
async function netUnixPacketListenCloseWhileIterating() {
const filePath = Deno.makeTempFileSync();
const filePath = tmpUnixSocketPath();
const socket = Deno.listenDatagram({
path: filePath,
transport: "unixpacket",
Expand Down Expand Up @@ -884,3 +891,18 @@ Deno.test(
clearTimeout(timer);
},
);

Deno.test({
ignore: Deno.build.os === "windows",
permissions: { read: true, write: true },
}, function netUnixListenAddrAlreadyInUse() {
const filePath = tmpUnixSocketPath();
const listener = Deno.listen({ path: filePath, transport: "unix" });
assertThrows(
() => {
Deno.listen({ path: filePath, transport: "unix" });
},
Deno.errors.AddrInUse,
);
listener.close();
});
4 changes: 1 addition & 3 deletions cli/tests/unit/remove_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -251,9 +251,7 @@ Deno.test(
Deno.statSync(path); // check if unix socket exists

await Deno[method](path);
assertThrows(() => {
Deno.statSync(path);
}, Deno.errors.NotFound);
assertThrows(() => Deno.statSync(path), Deno.errors.NotFound);
}
},
);
Expand Down
7 changes: 0 additions & 7 deletions ext/net/ops_unix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ use serde::Deserialize;
use serde::Serialize;
use std::borrow::Cow;
use std::cell::RefCell;
use std::fs::remove_file;
use std::path::Path;
use std::rc::Rc;
use tokio::net::UnixDatagram;
Expand Down Expand Up @@ -143,9 +142,6 @@ pub fn listen_unix(
state: &mut OpState,
addr: &Path,
) -> Result<(u32, tokio::net::unix::SocketAddr), AnyError> {
if addr.exists() {
remove_file(&addr)?;
}
let listener = UnixListener::bind(&addr)?;
let local_addr = listener.local_addr()?;
let listener_resource = UnixListenerResource {
Expand All @@ -161,9 +157,6 @@ pub fn listen_unix_packet(
state: &mut OpState,
addr: &Path,
) -> Result<(u32, tokio::net::unix::SocketAddr), AnyError> {
if addr.exists() {
remove_file(&addr)?;
}
let socket = UnixDatagram::bind(&addr)?;
let local_addr = socket.local_addr()?;
let datagram_resource = UnixDatagramResource {
Expand Down