Skip to content

Commit

Permalink
feat: disposable Deno resources (#20845)
Browse files Browse the repository at this point in the history
This commit implements Symbol.dispose and Symbol.asyncDispose for
the relevant resources.

Closes #20839

---------

Signed-off-by: Bartek Iwańczuk <biwanczuk@gmail.com>
Co-authored-by: Bartek Iwańczuk <biwanczuk@gmail.com>
  • Loading branch information
lucacasonato and bartlomieju committed Nov 1, 2023
1 parent 1d19b10 commit d42f154
Show file tree
Hide file tree
Showing 21 changed files with 342 additions and 35 deletions.
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 @@ -1749,7 +1749,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 @@ -1945,6 +1945,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

0 comments on commit d42f154

Please sign in to comment.