feat: deprecate Deno.resources() (#22059)

Most uses of `Deno.resources()` within tests, as they previously checked for leaked resources. This is not needed as the test runner does this automatically. Other internal uses of this API have been replaced with the internal `Deno[Deno.internal].core.resources()`.
2024-11-21 20:38:55 +00:00 · 2024-01-24 10:27:29 +11:00 · 2024-01-24 10:27:29 +11:00 · 947ce41e99
commit 947ce41e99
parent 4eedac3604
14 changed files with 32 additions and 111 deletions
--- a/cli/tests/testdata/run/fetch_response_finalization.js
+++ b/cli/tests/testdata/run/fetch_response_finalization.js
@ -1,6 +1,6 @@
 async function doAFetch() {
  const resp = await fetch("http://localhost:4545/README.md");
-  console.log(Deno.resources()); // print the current resources
+  console.log(Deno[Deno.internal].core.resources()); // print the current resources
  const _resp = resp;
  // at this point resp can be GC'ed
 }
@ -13,4 +13,4 @@ globalThis.gc(); // force GC
 // the response body is not called and the resource is not closed.
 await new Promise((resolve) => setTimeout(resolve, 0));

-console.log(Deno.resources()); // print the current resources
+console.log(Deno[Deno.internal].core.resources()); // print the current resources
--- a/cli/tests/unit/http_test.ts
+++ b/cli/tests/unit/http_test.ts
@ -1084,6 +1084,9 @@ Deno.test(
 );

 // https://github.com/denoland/deno/issues/11926
+// verify that the only new resource is "httpConnection", to make
+// sure "request" resource is closed even if its body was not read
+// by server handler
 Deno.test(
  { permissions: { net: true } },
  async function httpServerDoesntLeakResources2() {
@ -1105,22 +1108,11 @@ Deno.test(
      }
    })();

-    const resourcesBefore = Deno.resources();
    const response = await fetch(`http://127.0.0.1:${listenPort}`, {
      method: "POST",
      body: "hello world",
    });
    await response.text();
-    const resourcesAfter = Deno.resources();
-    // verify that the only new resource is "httpConnection", to make
-    // sure "request" resource is closed even if its body was not read
-    // by server handler
-
-    for (const rid of Object.keys(resourcesBefore)) {
-      delete resourcesAfter[Number(rid)];
-    }
-
-    assertEquals(Object.keys(resourcesAfter).length, 1);

    listener!.close();
    httpConn!.close();
--- a/cli/tests/unit/read_file_test.ts
+++ b/cli/tests/unit/read_file_test.ts
@ -75,18 +75,14 @@ Deno.test({ permissions: { read: true } }, function readFileSyncLoop() {
 Deno.test(
  { permissions: { read: true } },
  async function readFileDoesNotLeakResources() {
-    const resourcesBefore = Deno.resources();
    await assertRejects(async () => await Deno.readFile("cli"));
-    assertEquals(resourcesBefore, Deno.resources());
  },
 );

 Deno.test(
  { permissions: { read: true } },
  function readFileSyncDoesNotLeakResources() {
-    const resourcesBefore = Deno.resources();
    assertThrows(() => Deno.readFileSync("cli"));
-    assertEquals(resourcesBefore, Deno.resources());
  },
 );

--- a/cli/tests/unit/read_text_file_test.ts
+++ b/cli/tests/unit/read_text_file_test.ts
@ -73,18 +73,14 @@ Deno.test({ permissions: { read: true } }, function readTextFileSyncLoop() {
 Deno.test(
  { permissions: { read: true } },
  async function readTextFileDoesNotLeakResources() {
-    const resourcesBefore = Deno.resources();
    await assertRejects(async () => await Deno.readTextFile("cli"));
-    assertEquals(resourcesBefore, Deno.resources());
  },
 );

 Deno.test(
  { permissions: { read: true } },
  function readTextFileSyncDoesNotLeakResources() {
-    const resourcesBefore = Deno.resources();
    assertThrows(() => Deno.readTextFileSync("cli"));
-    assertEquals(resourcesBefore, Deno.resources());
  },
 );

--- a/cli/tests/unit_node/_fs/_fs_appendFile_test.ts
+++ b/cli/tests/unit_node/_fs/_fs_appendFile_test.ts
@ -97,7 +97,6 @@ Deno.test({
 Deno.test({
  name: "Async: Data is written to passed in file path",
  async fn() {
-    const openResourcesBeforeAppend: Deno.ResourceMap = Deno.resources();
    await new Promise<void>((resolve, reject) => {
      appendFile("_fs_appendFile_test_file.txt", "hello world", (err) => {
        if (err) reject(err);
@ -105,7 +104,6 @@ Deno.test({
      });
    })
      .then(async () => {
-        assertEquals(Deno.resources(), openResourcesBeforeAppend);
        const data = await Deno.readFile("_fs_appendFile_test_file.txt");
        assertEquals(decoder.decode(data), "hello world");
      }, (err) => {
@ -120,7 +118,6 @@ Deno.test({
 Deno.test({
  name: "Async: Data is written to passed in URL",
  async fn() {
-    const openResourcesBeforeAppend: Deno.ResourceMap = Deno.resources();
    const fileURL = new URL("_fs_appendFile_test_file.txt", import.meta.url);
    await new Promise<void>((resolve, reject) => {
      appendFile(fileURL, "hello world", (err) => {
@ -129,7 +126,6 @@ Deno.test({
      });
    })
      .then(async () => {
-        assertEquals(Deno.resources(), openResourcesBeforeAppend);
        const data = await Deno.readFile(fromFileUrl(fileURL));
        assertEquals(decoder.decode(data), "hello world");
      }, (err) => {
@ -145,7 +141,6 @@ Deno.test({
  name:
    "Async: Callback is made with error if attempting to append data to an existing file with 'ax' flag",
  async fn() {
-    const openResourcesBeforeAppend: Deno.ResourceMap = Deno.resources();
    const tempFile: string = await Deno.makeTempFile();
    await new Promise<void>((resolve, reject) => {
      appendFile(tempFile, "hello world", { flag: "ax" }, (err) => {
@ -153,11 +148,8 @@ Deno.test({
        else resolve();
      });
    })
-      .then(() => {
-        fail("Expected error to be thrown");
-      }, () => {
-        assertEquals(Deno.resources(), openResourcesBeforeAppend);
-      })
+      .then(() => fail("Expected error to be thrown"))
+      .catch(() => {})
      .finally(async () => {
        await Deno.remove(tempFile);
      });
@ -184,9 +176,7 @@ Deno.test({
 Deno.test({
  name: "Sync: Data is written to passed in file path",
  fn() {
-    const openResourcesBeforeAppend: Deno.ResourceMap = Deno.resources();
    appendFileSync("_fs_appendFile_test_file_sync.txt", "hello world");
-    assertEquals(Deno.resources(), openResourcesBeforeAppend);
    const data = Deno.readFileSync("_fs_appendFile_test_file_sync.txt");
    assertEquals(decoder.decode(data), "hello world");
    Deno.removeSync("_fs_appendFile_test_file_sync.txt");
@ -197,14 +187,12 @@ Deno.test({
  name:
    "Sync: error thrown if attempting to append data to an existing file with 'ax' flag",
  fn() {
-    const openResourcesBeforeAppend: Deno.ResourceMap = Deno.resources();
    const tempFile: string = Deno.makeTempFileSync();
    assertThrows(
      () => appendFileSync(tempFile, "hello world", { flag: "ax" }),
      Error,
      "",
    );
-    assertEquals(Deno.resources(), openResourcesBeforeAppend);
    Deno.removeSync(tempFile);
  },
 });
@ -212,10 +200,8 @@ Deno.test({
 Deno.test({
  name: "Sync: Data is written in Uint8Array to passed in file path",
  fn() {
-    const openResourcesBeforeAppend: Deno.ResourceMap = Deno.resources();
    const testData = new TextEncoder().encode("hello world");
    appendFileSync("_fs_appendFile_test_file_sync.txt", testData);
-    assertEquals(Deno.resources(), openResourcesBeforeAppend);
    const data = Deno.readFileSync("_fs_appendFile_test_file_sync.txt");
    assertEquals(data, testData);
    Deno.removeSync("_fs_appendFile_test_file_sync.txt");
@ -225,7 +211,6 @@ Deno.test({
 Deno.test({
  name: "Async: Data is written in Uint8Array to passed in file path",
  async fn() {
-    const openResourcesBeforeAppend: Deno.ResourceMap = Deno.resources();
    const testData = new TextEncoder().encode("hello world");
    await new Promise<void>((resolve, reject) => {
      appendFile("_fs_appendFile_test_file.txt", testData, (err) => {
@ -234,7 +219,6 @@ Deno.test({
      });
    })
      .then(async () => {
-        assertEquals(Deno.resources(), openResourcesBeforeAppend);
        const data = await Deno.readFile("_fs_appendFile_test_file.txt");
        assertEquals(data, testData);
      }, (err) => {
--- a/cli/tests/unit_node/_fs/_fs_close_test.ts
+++ b/cli/tests/unit_node/_fs/_fs_close_test.ts
@ -13,18 +13,13 @@ Deno.test({
    const tempFile: string = await Deno.makeTempFile();
    const file: Deno.FsFile = await Deno.open(tempFile);

-    assert(Deno.resources()[file.rid]);
    await new Promise<void>((resolve, reject) => {
      close(file.rid, (err) => {
        if (err !== null) reject();
        else resolve();
      });
    })
-      .then(() => {
-        assert(!Deno.resources()[file.rid]);
-      }, () => {
-        fail("No error expected");
-      })
+      .catch(() => fail("No error expected"))
      .finally(async () => {
        await Deno.remove(tempFile);
      });
@ -66,9 +61,7 @@ Deno.test({
    const tempFile: string = Deno.makeTempFileSync();
    const file: Deno.FsFile = Deno.openSync(tempFile);

-    assert(Deno.resources()[file.rid]);
    closeSync(file.rid);
-    assert(!Deno.resources()[file.rid]);
    Deno.removeSync(tempFile);
  },
 });
--- a/cli/tests/unit_node/_fs/_fs_open_test.ts
+++ b/cli/tests/unit_node/_fs/_fs_open_test.ts
@ -10,7 +10,6 @@ import {
  O_WRONLY,
 } from "node:constants";
 import {
-  assert,
  assertEquals,
  assertThrows,
  fail,
@ -35,7 +34,6 @@ Deno.test({
    })
      .then((fd) => {
        fd1 = fd;
-        assert(Deno.resources()[fd], `${fd}`);
      }, () => fail())
      .finally(() => closeSync(fd1));
  },
@ -46,7 +44,6 @@ Deno.test({
  fn() {
    const file = Deno.makeTempFileSync();
    const fd = openSync(file, "r");
-    assert(Deno.resources()[fd]);
    closeSync(fd);
  },
 });
@ -58,7 +55,6 @@ Deno.test({
    const fd = openSync(file, "a");
    assertEquals(typeof fd, "number");
    assertEquals(existsSync(file), true);
-    assert(Deno.resources()[fd]);
    closeSync(fd);
  },
 });
@ -225,7 +221,6 @@ Deno.test({
    const fd = openSync(file, O_APPEND | O_CREAT | O_WRONLY);
    assertEquals(typeof fd, "number");
    assertEquals(existsSync(file), true);
-    assert(Deno.resources()[fd]);
    closeSync(fd);
  },
 });
@ -403,7 +398,6 @@ Deno.test("[std/node/fs] open callback isn't called twice if error is thrown", a
    fn() {
      const file = Deno.makeTempFileSync();
      const fd = openSync(file, 0);
-      assert(Deno.resources()[fd]);
      closeSync(fd);
    },
  });
--- a/cli/tests/unit_node/_fs/_fs_rm_test.ts
+++ b/cli/tests/unit_node/_fs/_fs_rm_test.ts
@ -6,7 +6,7 @@ import {
  fail,
 } from "../../../../test_util/std/assert/mod.ts";
 import { rm, rmSync } from "node:fs";
-import { closeSync, existsSync } from "node:fs";
+import { existsSync } from "node:fs";
 import { join } from "../../../../test_util/std/path/mod.ts";

 Deno.test({
@ -26,27 +26,14 @@ Deno.test({
  },
 });

-function closeRes(before: Deno.ResourceMap, after: Deno.ResourceMap) {
-  for (const key in after) {
-    if (!before[key]) {
-      try {
-        closeSync(Number(key));
-      } catch (error) {
-        return error;
-      }
-    }
-  }
-}
-
 Deno.test({
  name: "ASYNC: removing non-empty folder",
  async fn() {
-    const rBefore = Deno.resources();
    const dir = Deno.makeTempDirSync();
-    Deno.createSync(join(dir, "file1.txt"));
-    Deno.createSync(join(dir, "file2.txt"));
+    using _file1 = Deno.createSync(join(dir, "file1.txt"));
+    using _file2 = Deno.createSync(join(dir, "file2.txt"));
    Deno.mkdirSync(join(dir, "some_dir"));
-    Deno.createSync(join(dir, "some_dir", "file.txt"));
+    using _file = Deno.createSync(join(dir, "some_dir", "file.txt"));
    await new Promise<void>((resolve, reject) => {
      rm(dir, { recursive: true }, (err) => {
        if (err) reject(err);
@ -56,8 +43,6 @@ Deno.test({
      .then(() => assertEquals(existsSync(dir), false), () => fail())
      .finally(() => {
        if (existsSync(dir)) Deno.removeSync(dir, { recursive: true });
-        const rAfter = Deno.resources();
-        closeRes(rBefore, rAfter);
      });
  },
  ignore: Deno.build.os === "windows",
@ -116,17 +101,13 @@ Deno.test({
 Deno.test({
  name: "SYNC: removing non-empty folder",
  fn() {
-    const rBefore = Deno.resources();
    const dir = Deno.makeTempDirSync();
-    Deno.createSync(join(dir, "file1.txt"));
-    Deno.createSync(join(dir, "file2.txt"));
+    using _file1 = Deno.createSync(join(dir, "file1.txt"));
+    using _file2 = Deno.createSync(join(dir, "file2.txt"));
    Deno.mkdirSync(join(dir, "some_dir"));
-    Deno.createSync(join(dir, "some_dir", "file.txt"));
+    using _file = Deno.createSync(join(dir, "some_dir", "file.txt"));
    rmSync(dir, { recursive: true });
    assertEquals(existsSync(dir), false);
-    // closing resources
-    const rAfter = Deno.resources();
-    closeRes(rBefore, rAfter);
  },
  ignore: Deno.build.os === "windows",
 });
--- a/cli/tests/unit_node/_fs/_fs_rmdir_test.ts
+++ b/cli/tests/unit_node/_fs/_fs_rmdir_test.ts
@ -1,7 +1,6 @@
 // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license.
 import { assertEquals, fail } from "../../../../test_util/std/assert/mod.ts";
 import { rmdir, rmdirSync } from "node:fs";
-import { closeSync } from "node:fs";
 import { existsSync } from "node:fs";
 import { join } from "../../../../test_util/std/path/mod.ts";
 import { assertCallbackErrorUncaught } from "../_test_utils.ts";
@ -32,27 +31,14 @@ Deno.test({
  },
 });

-function closeRes(before: Deno.ResourceMap, after: Deno.ResourceMap) {
-  for (const key in after) {
-    if (!before[key]) {
-      try {
-        closeSync(Number(key));
-      } catch (error) {
-        return error;
-      }
-    }
-  }
-}
-
 Deno.test({
  name: "ASYNC: removing non-empty folder",
  async fn() {
-    const rBefore = Deno.resources();
    const dir = Deno.makeTempDirSync();
-    Deno.createSync(join(dir, "file1.txt"));
-    Deno.createSync(join(dir, "file2.txt"));
+    using _file1 = Deno.createSync(join(dir, "file1.txt"));
+    using _file2 = Deno.createSync(join(dir, "file2.txt"));
    Deno.mkdirSync(join(dir, "some_dir"));
-    Deno.createSync(join(dir, "some_dir", "file.txt"));
+    using _file = Deno.createSync(join(dir, "some_dir", "file.txt"));
    await new Promise<void>((resolve, reject) => {
      rmdir(dir, { recursive: true }, (err) => {
        if (err) reject(err);
@ -62,8 +48,6 @@ Deno.test({
      .then(() => assertEquals(existsSync(dir), false), () => fail())
      .finally(() => {
        if (existsSync(dir)) Deno.removeSync(dir, { recursive: true });
-        const rAfter = Deno.resources();
-        closeRes(rBefore, rAfter);
      });
  },
  ignore: Deno.build.os === "windows",
@ -72,17 +56,13 @@ Deno.test({
 Deno.test({
  name: "SYNC: removing non-empty folder",
  fn() {
-    const rBefore = Deno.resources();
    const dir = Deno.makeTempDirSync();
-    Deno.createSync(join(dir, "file1.txt"));
-    Deno.createSync(join(dir, "file2.txt"));
+    using _file1 = Deno.createSync(join(dir, "file1.txt"));
+    using _file2 = Deno.createSync(join(dir, "file2.txt"));
    Deno.mkdirSync(join(dir, "some_dir"));
-    Deno.createSync(join(dir, "some_dir", "file.txt"));
+    using _file = Deno.createSync(join(dir, "some_dir", "file.txt"));
    rmdirSync(dir, { recursive: true });
    assertEquals(existsSync(dir), false);
-    // closing resources
-    const rAfter = Deno.resources();
-    closeRes(rBefore, rAfter);
  },
  ignore: Deno.build.os === "windows",
 });
--- a/cli/tsc/dts/lib.deno.ns.d.ts
+++ b/cli/tsc/dts/lib.deno.ns.d.ts
@ -3889,6 +3889,8 @@ declare namespace Deno {
   * A map of open resources that Deno is tracking. The key is the resource ID
   * (_rid_) and the value is its representation.
   *
+   * @deprecated {@linkcode Deno.resources} will be removed in Deno 2.0.
+   *
   * @category Observability */
  interface ResourceMap {
    [rid: number]: unknown;
@ -3907,6 +3909,8 @@ declare namespace Deno {
   * // { 0: "stdin", 1: "stdout", 2: "stderr", 3: "fsFile" }
   * ```
   *
+   * @deprecated {@linkcode Deno.resources} will be removed in Deno 2.0.
+   *
   * @category Observability
   */
  export function resources(): ResourceMap;
--- a/ext/node/polyfills/http2.ts
+++ b/ext/node/polyfills/http2.ts
@ -316,7 +316,7 @@ function closeSession(session: Http2Session, code?: number, error?: Error) {
    session[kDenoConnRid],
    session[kDenoClientRid],
  );
-  console.table(Deno.resources());
+  console.table(Deno[Deno.internal].core.resources());
  if (session[kDenoConnRid]) {
    core.tryClose(session[kDenoConnRid]);
  }
--- a/runtime/js/99_main.js
+++ b/runtime/js/99_main.js
@ -534,7 +534,10 @@ const internalSymbol = Symbol("Deno.internal");
 const finalDenoNs = {
  internal: internalSymbol,
  [internalSymbol]: internals,
-  resources: core.resources,
+  resources() {
+    internals.warnOnDeprecatedApi("Deno.resources()", new Error().stack);
+    return core.resources();
+  },
  close: core.close,
  ...denoNs,
  // Deno.test and Deno.bench are noops here, but kept for compatibility; so
--- a/test_ffi/tests/test.js
+++ b/test_ffi/tests/test.js
@ -20,7 +20,7 @@ const [libPrefix, libSuffix] = {
 }[Deno.build.os];
 const libPath = `${targetDir}/${libPrefix}test_ffi.${libSuffix}`;

-const resourcesPre = Deno.resources();
+const resourcesPre = Deno[Deno.internal].core.resources();

 // dlopen shouldn't panic
 assertThrows(() => {
@ -765,7 +765,7 @@ testOptimized(hash, () => hash());
  nestedCallback.close();
  addToFooCallback.close();

-  const resourcesPost = Deno.resources();
+  const resourcesPost = Deno[Deno.internal].core.resources();

  const preStr = JSON.stringify(resourcesPre, null, 2);
  const postStr = JSON.stringify(resourcesPost, null, 2);
--- a/test_ffi/tests/thread_safe_test.js
+++ b/test_ffi/tests/thread_safe_test.js
@ -9,8 +9,6 @@ const [libPrefix, libSuffix] = {
 }[Deno.build.os];
 const libPath = `${targetDir}/${libPrefix}test_ffi.${libSuffix}`;

-const resourcesPre = Deno.resources();
-
 const dylib = Deno.dlopen(libPath, {
  store_function: {
    parameters: ["function"],