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

feat: disposable Deno resources #20845

Merged
merged 11 commits into from
Nov 1, 2023
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
40 changes: 40 additions & 0 deletions cli/tests/unit/command_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,46 @@ Deno.test(
},
);

Deno.test(
{ permissions: { run: true, read: true } },
// deno lint bug, see https://github.com/denoland/deno_lint/issues/1206
// deno-lint-ignore require-await
async function childProcessExplicitResourceManagement() {
let dead = undefined;
{
const command = new Deno.Command(Deno.execPath(), {
args: ["eval", "setTimeout(() => {}, 10000)"],
stdout: "null",
stderr: "null",
});
await using child = command.spawn();
child.status.then(({ signal }) => {
dead = signal;
});
}

if (Deno.build.os == "windows") {
assertEquals(dead, null);
} else {
assertEquals(dead, "SIGTERM");
}
},
);

Deno.test(
{ permissions: { run: true, read: true } },
async function childProcessExplicitResourceManagementManualClose() {
const command = new Deno.Command(Deno.execPath(), {
args: ["eval", "setTimeout(() => {}, 10000)"],
stdout: "null",
stderr: "null",
});
await using child = command.spawn();
child.kill("SIGTERM");
await child.status;
},
);

Deno.test(
{ permissions: { run: true, read: true } },
async function commandKillFailed() {
Expand Down
27 changes: 27 additions & 0 deletions cli/tests/unit/files_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -824,3 +824,30 @@ Deno.test(
assertEquals(res, "hello \uFFFD");
},
);

Deno.test(
{ permissions: { read: true } },
async function fsFileExplicitResourceManagement() {
let file2: Deno.FsFile;

{
using file = await Deno.open("cli/tests/testdata/assets/hello.txt");
file2 = file;

const stat = file.statSync();
assert(stat.isFile);
}

assertThrows(() => file2.statSync(), Deno.errors.BadResource);
},
);

Deno.test(
{ permissions: { read: true } },
async function fsFileExplicitResourceManagementManualClose() {
using file = await Deno.open("cli/tests/testdata/assets/hello.txt");
file.close();
assertThrows(() => file.statSync(), Deno.errors.BadResource); // definitely closed
// calling [Symbol.dispose] after manual close is a no-op
},
);
30 changes: 30 additions & 0 deletions cli/tests/unit/fs_events_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,3 +107,33 @@ Deno.test(
assertEquals(events, []);
},
);

Deno.test(
{ permissions: { read: true, write: true } },
async function watchFsExplicitResourceManagement() {
let res;
{
const testDir = await makeTempDir();
using iter = Deno.watchFs(testDir);

res = iter[Symbol.asyncIterator]().next();
}

const { done } = await res;
assert(done);
},
);

Deno.test(
{ permissions: { read: true, write: true } },
async function watchFsExplicitResourceManagementManualClose() {
const testDir = await makeTempDir();
using iter = Deno.watchFs(testDir);

const res = iter[Symbol.asyncIterator]().next();

iter.close();
const { done } = await res;
assert(done);
},
);
18 changes: 18 additions & 0 deletions cli/tests/unit/http_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2817,6 +2817,24 @@ Deno.test({
},
});

Deno.test(
async function httpConnExplicitResourceManagement() {
let promise;

{
const listen = Deno.listen({ port: listenPort });
promise = fetch(`http://localhost:${listenPort}/`).catch(() => null);
const serverConn = await listen.accept();
listen.close();

using _httpConn = Deno.serveHttp(serverConn);
}

const response = await promise;
assertEquals(response, null);
},
);

function chunkedBodyReader(h: Headers, r: BufReader): Deno.Reader {
// Based on https://tools.ietf.org/html/rfc2616#section-19.4.6
const tp = new TextProtoReader(r);
Expand Down
27 changes: 27 additions & 0 deletions cli/tests/unit/kv_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2100,3 +2100,30 @@ Deno.test({
db.close();
},
});

Deno.test(
{ permissions: { read: true } },
async function kvExplicitResourceManagement() {
let kv2: Deno.Kv;

{
using kv = await Deno.openKv(":memory:");
kv2 = kv;

const res = await kv.get(["a"]);
assertEquals(res.versionstamp, null);
}

await assertRejects(() => kv2.get(["a"]), Deno.errors.BadResource);
},
);

Deno.test(
{ permissions: { read: true } },
async function kvExplicitResourceManagementManualClose() {
using kv = await Deno.openKv(":memory:");
kv.close();
await assertRejects(() => kv.get(["a"]), Deno.errors.BadResource);
// calling [Symbol.dispose] after manual close is a no-op
},
);
31 changes: 31 additions & 0 deletions cli/tests/unit/net_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1242,3 +1242,34 @@ Deno.test({
const listener = Deno.listen({ hostname: "localhost", port: "0" });
listener.close();
});

Deno.test(
{ permissions: { net: true } },
async function listenerExplicitResourceManagement() {
let done: Promise<Deno.errors.BadResource>;

{
using listener = Deno.listen({ port: listenPort });

done = assertRejects(
() => listener.accept(),
Deno.errors.BadResource,
);
}

await done;
},
);

Deno.test(
{ permissions: { net: true } },
async function listenerExplicitResourceManagementManualClose() {
using listener = Deno.listen({ port: listenPort });
listener.close();
await assertRejects( // definitely closed
() => listener.accept(),
Deno.errors.BadResource,
);
// calling [Symbol.dispose] after manual close is a no-op
},
);
46 changes: 45 additions & 1 deletion cli/tests/unit/serve_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,12 @@ function onListen<T>(
async function makeServer(
handler: (req: Request) => Response | Promise<Response>,
): Promise<
{ finished: Promise<void>; abort: () => void; shutdown: () => Promise<void> }
{
finished: Promise<void>;
abort: () => void;
shutdown: () => Promise<void>;
[Symbol.asyncDispose](): PromiseLike<void>;
}
> {
const ac = new AbortController();
const listeningPromise = deferred();
Expand All @@ -69,6 +74,9 @@ async function makeServer(
async shutdown() {
await server.shutdown();
},
[Symbol.asyncDispose]() {
return server[Symbol.asyncDispose]();
},
};
}

Expand Down Expand Up @@ -296,6 +304,42 @@ Deno.test(
},
);

Deno.test(
{ permissions: { net: true, write: true, read: true } },
async function httpServerExplicitResourceManagement() {
let dataPromise;

{
await using _server = await makeServer(async (_req) => {
return new Response((await makeTempFile(1024 * 1024)).readable);
});

const resp = await fetch(`http://localhost:${servePort}`);
dataPromise = resp.arrayBuffer();
}

assertEquals((await dataPromise).byteLength, 1048576);
},
);

Deno.test(
{ permissions: { net: true, write: true, read: true } },
async function httpServerExplicitResourceManagementManualClose() {
await using server = await makeServer(async (_req) => {
return new Response((await makeTempFile(1024 * 1024)).readable);
});

const resp = await fetch(`http://localhost:${servePort}`);

const [_, data] = await Promise.all([
server.shutdown(),
resp.arrayBuffer(),
]);

assertEquals(data.byteLength, 1048576);
},
);

Deno.test(
{ permissions: { read: true, run: true } },
async function httpServerUnref() {
Expand Down
6 changes: 5 additions & 1 deletion cli/tsc/99_main_compiler.js
Original file line number Diff line number Diff line change
Expand Up @@ -1138,6 +1138,7 @@ delete Object.prototype.__proto__;
`${ASSETS_URL_PREFIX}${specifier}`,
ts.ScriptTarget.ESNext,
),
`failed to load '${ASSETS_URL_PREFIX}${specifier}'`,
);
}
// this helps ensure as much as possible is in memory that is re-usable
Expand All @@ -1148,7 +1149,10 @@ delete Object.prototype.__proto__;
options: SNAPSHOT_COMPILE_OPTIONS,
host,
});
assert(ts.getPreEmitDiagnostics(TS_SNAPSHOT_PROGRAM).length === 0);
assert(
ts.getPreEmitDiagnostics(TS_SNAPSHOT_PROGRAM).length === 0,
"lib.d.ts files have errors",
);

// remove this now that we don't need it anymore for warming up tsc
sourceFileCache.delete(buildSpecifier);
Expand Down
16 changes: 11 additions & 5 deletions cli/tsc/dts/lib.deno.ns.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

/// <reference no-default-lib="true" />
/// <reference lib="esnext" />
/// <reference lib="esnext.disposable" />
/// <reference lib="deno.net" />

/** Deno provides extra properties on `import.meta`. These are included here
Expand Down Expand Up @@ -2177,7 +2178,8 @@ declare namespace Deno {
WriterSync,
Seeker,
SeekerSync,
Closer {
Closer,
Disposable {
/** The resource ID associated with the file instance. The resource ID
* should be considered an opaque reference to resource. */
readonly rid: number;
Expand Down Expand Up @@ -2451,6 +2453,8 @@ declare namespace Deno {
* ```
*/
close(): void;

[Symbol.dispose](): void;
}

/**
Expand Down Expand Up @@ -3831,7 +3835,7 @@ declare namespace Deno {
*
* @category File System
*/
export interface FsWatcher extends AsyncIterable<FsEvent> {
export interface FsWatcher extends AsyncIterable<FsEvent>, Disposable {
/** The resource id. */
readonly rid: number;
/** Stops watching the file system and closes the watcher resource. */
Expand Down Expand Up @@ -4284,7 +4288,7 @@ declare namespace Deno {
*
* @category Sub Process
*/
export class ChildProcess {
export class ChildProcess implements Disposable {
get stdin(): WritableStream<Uint8Array>;
get stdout(): ReadableStream<Uint8Array>;
get stderr(): ReadableStream<Uint8Array>;
Expand All @@ -4307,6 +4311,8 @@ declare namespace Deno {
/** Ensure that the status of the child process does not block the Deno
* process from exiting. */
unref(): void;

[Symbol.dispose](): void;
}

/**
Expand Down Expand Up @@ -5258,7 +5264,7 @@ declare namespace Deno {
* requests on the HTTP server connection.
*
* @category HTTP Server */
export interface HttpConn extends AsyncIterable<RequestEvent> {
export interface HttpConn extends AsyncIterable<RequestEvent>, Disposable {
/** The resource ID associated with this connection. Generally users do not
* need to be aware of this identifier. */
readonly rid: number;
Expand Down Expand Up @@ -5911,7 +5917,7 @@ declare namespace Deno {
*
* @category HTTP Server
*/
export interface HttpServer {
export interface HttpServer extends AsyncDisposable {
/** A promise that resolves once server finishes - eg. when aborted using
* the signal passed to {@linkcode ServeOptions.signal}.
*/
Expand Down
4 changes: 3 additions & 1 deletion cli/tsc/dts/lib.deno.unstable.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1724,7 +1724,7 @@ declare namespace Deno {
*
* @category KV
*/
export class Kv {
export class Kv implements Disposable {
/**
* Retrieve the value and versionstamp for the given key from the database
* in the form of a {@linkcode Deno.KvEntryMaybe}. If no value exists for
Expand Down Expand Up @@ -1920,6 +1920,8 @@ declare namespace Deno {
* operations immediately.
*/
close(): void;

[Symbol.dispose](): void;
}

/** **UNSTABLE**: New API, yet to be vetted.
Expand Down
6 changes: 5 additions & 1 deletion ext/fs/30_fs.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ import {
ReadableStreamPrototype,
writableStreamForRid,
} from "ext:deno_web/06_streams.js";
import { pathFromURL } from "ext:deno_web/00_infra.js";
import { pathFromURL, SymbolDispose } from "ext:deno_web/00_infra.js";

function chmodSync(path, mode) {
ops.op_fs_chmod_sync(pathFromURL(path), mode);
Expand Down Expand Up @@ -669,6 +669,10 @@ class FsFile {
}
return this.#writable;
}

[SymbolDispose]() {
core.tryClose(this.rid);
}
}

function checkOpenOptions(options) {
Expand Down
Loading