feat: disposable Deno resources (#20845)

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>
This commit is contained in:
Luca Casonato 2023-11-01 20:26:12 +01:00 committed by GitHub
parent 1d19b1011b
commit d42f154312
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
21 changed files with 342 additions and 35 deletions

View file

@ -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() {

View file

@ -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
},
);

View file

@ -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);
},
);

View file

@ -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);

View file

@ -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
},
);

View file

@ -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
},
);

View file

@ -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();
@ -69,6 +74,9 @@ async function makeServer(
async shutdown() {
await server.shutdown();
},
[Symbol.asyncDispose]() {
return server[Symbol.asyncDispose]();
},
};
}
@ -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() {

View file

@ -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
@ -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);

View file

@ -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
@ -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;
@ -2451,6 +2453,8 @@ declare namespace Deno {
* ```
*/
close(): void;
[Symbol.dispose](): void;
}
/**
@ -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. */
@ -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>;
@ -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;
}
/**
@ -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;
@ -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}.
*/

View file

@ -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
@ -1945,6 +1945,8 @@ declare namespace Deno {
* operations immediately.
*/
close(): void;
[Symbol.dispose](): void;
}
/** **UNSTABLE**: New API, yet to be vetted.

View file

@ -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);
@ -669,6 +669,10 @@ class FsFile {
}
return this.#writable;
}
[SymbolDispose]() {
core.tryClose(this.rid);
}
}
function checkOpenOptions(options) {

View file

@ -36,6 +36,7 @@ import {
} from "ext:deno_web/06_streams.js";
import { listen, listenOptionApiName, TcpConn } from "ext:deno_net/01_net.js";
import { listenTls } from "ext:deno_net/02_tls.js";
import { SymbolAsyncDispose } from "ext:deno_web/00_infra.js";
const {
ArrayPrototypePush,
ObjectHasOwn,
@ -343,6 +344,7 @@ class CallbackContext {
fallbackHost;
serverRid;
closed;
/** @type {Promise<void> | undefined} */
closing;
listener;
@ -671,22 +673,25 @@ function serveHttpOn(context, callback) {
PromisePrototypeCatch(callback(req), promiseErrorHandler);
}
if (!context.closed && !context.closing) {
context.closed = true;
await op_http_close(rid, false);
if (!context.closing && !context.closed) {
context.closing = op_http_close(rid, false);
context.close();
}
await context.closing;
context.close();
context.closed = true;
})();
return {
finished,
async shutdown() {
if (!context.closed && !context.closing) {
if (!context.closing && !context.closed) {
// Shut this HTTP server down gracefully
context.closing = true;
await op_http_close(context.serverRid, true);
context.closed = true;
context.closing = op_http_close(context.serverRid, true);
}
await context.closing;
context.closed = true;
},
ref() {
ref = true;
@ -700,6 +705,9 @@ function serveHttpOn(context, callback) {
core.unrefOp(currentPromise[promiseIdSymbol]);
}
},
[SymbolAsyncDispose]() {
return this.shutdown();
},
};
}

View file

@ -44,6 +44,7 @@ import {
ReadableStreamPrototype,
} from "ext:deno_web/06_streams.js";
import { serve } from "ext:deno_http/00_serve.js";
import { SymbolDispose } from "ext:deno_web/00_infra.js";
const {
ArrayPrototypeIncludes,
ArrayPrototypeMap,
@ -69,6 +70,9 @@ const {
const connErrorSymbol = Symbol("connError");
const _deferred = Symbol("upgradeHttpDeferred");
/** @type {(self: HttpConn, rid: number) => boolean} */
let deleteManagedResource;
class HttpConn {
#rid = 0;
#closed = false;
@ -79,7 +83,12 @@ class HttpConn {
// that were created during lifecycle of this request.
// When the connection is closed these resources should be closed
// as well.
managedResources = new SafeSet();
#managedResources = new SafeSet();
static {
deleteManagedResource = (self, rid) =>
SetPrototypeDelete(self.#managedResources, rid);
}
constructor(rid, remoteAddr, localAddr) {
this.#rid = rid;
@ -123,7 +132,7 @@ class HttpConn {
}
const { 0: streamRid, 1: method, 2: url } = nextRequest;
SetPrototypeAdd(this.managedResources, streamRid);
SetPrototypeAdd(this.#managedResources, streamRid);
/** @type {ReadableStream<Uint8Array> | undefined} */
let body = null;
@ -167,13 +176,21 @@ class HttpConn {
if (!this.#closed) {
this.#closed = true;
core.close(this.#rid);
for (const rid of new SafeSetIterator(this.managedResources)) {
SetPrototypeDelete(this.managedResources, rid);
for (const rid of new SafeSetIterator(this.#managedResources)) {
SetPrototypeDelete(this.#managedResources, rid);
core.close(rid);
}
}
}
[SymbolDispose]() {
core.tryClose(this.#rid);
for (const rid of new SafeSetIterator(this.#managedResources)) {
SetPrototypeDelete(this.#managedResources, rid);
core.tryClose(rid);
}
}
[SymbolAsyncIterator]() {
// deno-lint-ignore no-this-alias
const httpConn = this;
@ -395,7 +412,7 @@ function createRespondWith(
abortController.abort(error);
throw error;
} finally {
if (SetPrototypeDelete(httpConn.managedResources, streamRid)) {
if (deleteManagedResource(httpConn, streamRid)) {
core.close(streamRid);
}
}

View file

@ -12,6 +12,7 @@ const {
SymbolToStringTag,
Uint8ArrayPrototype,
} = globalThis.__bootstrap.primordials;
import { SymbolDispose } from "ext:deno_web/00_infra.js";
const core = Deno.core;
const ops = core.ops;
@ -299,6 +300,10 @@ class Kv {
close() {
core.close(this.#rid);
}
[SymbolDispose]() {
core.tryClose(this.#rid);
}
}
class AtomicOperation {

View file

@ -66,7 +66,7 @@ const MAX_TOTAL_MUTATION_SIZE_BYTES: usize = 800 * 1024;
const MAX_TOTAL_KEY_SIZE_BYTES: usize = 80 * 1024;
deno_core::extension!(deno_kv,
deps = [ deno_console ],
deps = [ deno_console, deno_web ],
parameters = [ DBH: DatabaseHandler ],
ops = [
op_kv_database_open<DBH>,

View file

@ -9,6 +9,8 @@ import {
writableStreamForRid,
} from "ext:deno_web/06_streams.js";
import * as abortSignal from "ext:deno_web/03_abort_signal.js";
import { SymbolDispose } from "ext:deno_web/00_infra.js";
const primordials = globalThis.__bootstrap.primordials;
const {
ArrayPrototypeFilter,
@ -160,6 +162,10 @@ class Conn {
(id) => core.unrefOp(id),
);
}
[SymbolDispose]() {
core.tryClose(this.#rid);
}
}
class TcpConn extends Conn {
@ -249,6 +255,10 @@ class Listener {
core.close(this.rid);
}
[SymbolDispose]() {
core.tryClose(this.#rid);
}
[SymbolAsyncIterator]() {
return this;
}

View file

@ -2,6 +2,7 @@
/// <reference no-default-lib="true" />
/// <reference lib="esnext" />
/// <reference lib="esnext.disposable" />
declare namespace Deno {
/** @category Network */
@ -24,7 +25,8 @@ declare namespace Deno {
*
* @category Network
*/
export interface Listener<T extends Conn = Conn> extends AsyncIterable<T> {
export interface Listener<T extends Conn = Conn>
extends AsyncIterable<T>, Disposable {
/** Waits for and resolves to the next connection to the `Listener`. */
accept(): Promise<T>;
/** Close closes the listener. Any pending accept promises will be rejected
@ -57,7 +59,7 @@ declare namespace Deno {
export type TlsListener = Listener<TlsConn>;
/** @category Network */
export interface Conn extends Reader, Writer, Closer {
export interface Conn extends Reader, Writer, Closer, Disposable {
/** The local address of the connection. */
readonly localAddr: Addr;
/** The remote address of the connection. */

View file

@ -32,6 +32,7 @@ const {
StringPrototypeSubstring,
StringPrototypeToLowerCase,
StringPrototypeToUpperCase,
Symbol,
TypeError,
} = primordials;
import { URLPrototype } from "ext:deno_url/00_url.js";
@ -458,6 +459,12 @@ function pathFromURL(pathOrUrl) {
// it in unit tests
internals.pathFromURL = pathFromURL;
// deno-lint-ignore prefer-primordials
export const SymbolDispose = Symbol.dispose ?? Symbol("Symbol.dispose");
// deno-lint-ignore prefer-primordials
export const SymbolAsyncDispose = Symbol.asyncDispose ??
Symbol("Symbol.asyncDispose");
export {
ASCII_ALPHA,
ASCII_ALPHANUMERIC,

View file

@ -9,6 +9,8 @@ const {
PromiseResolve,
SymbolAsyncIterator,
} = primordials;
import { SymbolDispose } from "ext:deno_web/00_infra.js";
class FsWatcher {
#rid = 0;
@ -51,6 +53,10 @@ class FsWatcher {
[SymbolAsyncIterator]() {
return this;
}
[SymbolDispose]() {
core.tryClose(this.#rid);
}
}
function watchFs(

View file

@ -18,7 +18,11 @@ const {
} = primordials;
import { FsFile } from "ext:deno_fs/30_fs.js";
import { readAll } from "ext:deno_io/12_io.js";
import { assert, pathFromURL } from "ext:deno_web/00_infra.js";
import {
assert,
pathFromURL,
SymbolAsyncDispose,
} from "ext:deno_web/00_infra.js";
import * as abortSignal from "ext:deno_web/03_abort_signal.js";
import {
readableStreamCollectIntoUint8Array,
@ -201,7 +205,6 @@ class ChildProcess {
#rid;
#waitPromiseId;
#waitComplete = false;
#unrefed = false;
#pid;
get pid() {
@ -216,7 +219,6 @@ class ChildProcess {
return this.#stdin;
}
#stdoutRid;
#stdout = null;
get stdout() {
if (this.#stdout == null) {
@ -225,7 +227,6 @@ class ChildProcess {
return this.#stdout;
}
#stderrRid;
#stderr = null;
get stderr() {
if (this.#stderr == null) {
@ -254,12 +255,10 @@ class ChildProcess {
}
if (stdoutRid !== null) {
this.#stdoutRid = stdoutRid;
this.#stdout = readableStreamForRidUnrefable(stdoutRid);
}
if (stderrRid !== null) {
this.#stderrRid = stderrRid;
this.#stderr = readableStreamForRidUnrefable(stderrRid);
}
@ -324,15 +323,22 @@ class ChildProcess {
ops.op_spawn_kill(this.#rid, signo);
}
async [SymbolAsyncDispose]() {
try {
ops.op_spawn_kill(this.#rid, "SIGTERM");
} catch {
// ignore errors from killing the process (such as ESRCH or BadResource)
}
await this.#status;
}
ref() {
this.#unrefed = false;
core.refOp(this.#waitPromiseId);
if (this.#stdout) readableStreamForRidUnrefableRef(this.#stdout);
if (this.#stderr) readableStreamForRidUnrefableRef(this.#stderr);
}
unref() {
this.#unrefed = true;
core.unrefOp(this.#waitPromiseId);
if (this.#stdout) readableStreamForRidUnrefableUnref(this.#stdout);
if (this.#stderr) readableStreamForRidUnrefableUnref(this.#stderr);

View file

@ -70,11 +70,24 @@ import {
windowOrWorkerGlobalScope,
workerRuntimeGlobalProperties,
} from "ext:runtime/98_global_scope.js";
import { SymbolAsyncDispose, SymbolDispose } from "ext:deno_web/00_infra.js";
// deno-lint-ignore prefer-primordials
Symbol.dispose ??= Symbol("Symbol.dispose");
// deno-lint-ignore prefer-primordials
Symbol.asyncDispose ??= Symbol("Symbol.asyncDispose");
if (Symbol.dispose) throw "V8 supports Symbol.dispose now, no need to shim it!";
ObjectDefineProperties(Symbol, {
dispose: {
value: SymbolDispose,
enumerable: false,
writable: false,
configurable: false,
},
asyncDispose: {
value: SymbolAsyncDispose,
enumerable: false,
writable: false,
configurable: false,
},
});
let windowIsClosing = false;
let globalThis_;