Merge pull request #706 from denoland/main

Create a new pull request by comparing changes across two branches

Author: GulajavaMinistudio

ext/cache/lib.rs

@@ -33,7 +33,9 @@ pub enum CacheError {
 }
 
 #[derive(Clone)]
-pub struct CreateCache<C: Cache + 'static>(pub Arc<dyn Fn() -> C>);
+pub struct CreateCache<C: Cache + 'static>(
+  pub Arc<dyn Fn() -> Result<C, CacheError>>,
+);
 
 deno_core::extension!(deno_cache,
   deps = [ deno_webidl, deno_web, deno_url, deno_fetch ],
@@ -231,7 +233,7 @@ where
   if let Some(cache) = state.try_borrow::<CA>() {
     Ok(cache.clone())
   } else if let Some(create_cache) = state.try_borrow::<CreateCache<CA>>() {
-    let cache = create_cache.0();
+    let cache = create_cache.0()?;
     state.put(cache);
     Ok(state.borrow::<CA>().clone())
   } else {

ext/cache/sqlite.rs

@@ -42,7 +42,7 @@ pub struct SqliteBackedCache {
 }
 
 impl SqliteBackedCache {
-  pub fn new(cache_storage_dir: PathBuf) -> Self {
+  pub fn new(cache_storage_dir: PathBuf) -> Result<Self, CacheError> {
     {
       std::fs::create_dir_all(&cache_storage_dir)
         .expect("failed to create cache dir");
@@ -57,18 +57,14 @@ impl SqliteBackedCache {
         PRAGMA synchronous=NORMAL;
         PRAGMA optimize;
       ";
-      connection
-        .execute_batch(initial_pragmas)
-        .expect("failed to execute pragmas");
-      connection
-        .execute(
-          "CREATE TABLE IF NOT EXISTS cache_storage (
+      connection.execute_batch(initial_pragmas)?;
+      connection.execute(
+        "CREATE TABLE IF NOT EXISTS cache_storage (
                     id              INTEGER PRIMARY KEY,
                     cache_name      TEXT NOT NULL UNIQUE
                 )",
-          (),
-        )
-        .expect("failed to create cache_storage table");
+        (),
+      )?;
       connection
         .execute(
           "CREATE TABLE IF NOT EXISTS request_response_list (
@@ -86,12 +82,11 @@ impl SqliteBackedCache {
                     UNIQUE (cache_id, request_url)
                 )",
           (),
-        )
-        .expect("failed to create request_response_list table");
-      SqliteBackedCache {
+        )?;
+      Ok(SqliteBackedCache {
         connection: Arc::new(Mutex::new(connection)),
         cache_storage_dir,
-      }
+      })
     }
   }
 }

ext/fetch/23_request.js

@@ -269,19 +269,25 @@ class Request {
   /** @type {AbortSignal} */
   get [_signal]() {
     const signal = this[_signalCache];
-    // This signal not been created yet, and the request is still in progress
-    if (signal === undefined) {
+    // This signal has not been created yet, but the request has already completed
+    if (signal === false) {
       const signal = newSignal();
       this[_signalCache] = signal;
+      signal[signalAbort](signalAbortError);
       return signal;
     }
-    // This signal has not been created yet, but the request has already completed
-    if (signal === false) {
+
+    // This signal not been created yet, and the request is still in progress
+    if (signal === undefined) {
       const signal = newSignal();
       this[_signalCache] = signal;
-      signal[signalAbort](signalAbortError);
       return signal;
     }
+
+    if (!signal.aborted && this[_request].isCancelled) {
+      signal[signalAbort](signalAbortError);
+    }
+
     return signal;
   }
   get [_mimeType]() {

ext/http/00_serve.ts

@@ -11,6 +11,7 @@ import {
   op_http_cancel,
   op_http_close,
   op_http_close_after_finish,
+  op_http_get_request_cancelled,
   op_http_get_request_headers,
   op_http_get_request_method_and_url,
   op_http_read_request_body,
@@ -373,6 +374,13 @@ class InnerRequest {
   get external() {
     return this.#external;
   }
+
+  get isCancelled() {
+    if (this.#external === null) {
+      return true;
+    }
+    return op_http_get_request_cancelled(this.#external);
+  }
 }
 
 class CallbackContext {

ext/http/http_next.rs

@@ -700,6 +700,14 @@ fn set_response(
   http.complete();
 }
 
+#[op2(fast)]
+pub fn op_http_get_request_cancelled(external: *const c_void) -> bool {
+  let http =
+    // SAFETY: op is called with external.
+    unsafe { clone_external!(external, "op_http_get_request_cancelled") };
+  http.cancelled()
+}
+
 /// Returned promise resolves when body streaming finishes.
 /// Call [`op_http_close_after_finish`] when done with the external.
 #[op2(async)]

ext/http/lib.rs

@@ -113,6 +113,7 @@ deno_core::extension!(
     http_next::op_http_get_request_header,
     http_next::op_http_get_request_headers,
     http_next::op_http_get_request_method_and_url<HTTP>,
+    http_next::op_http_get_request_cancelled,
     http_next::op_http_read_request_body,
     http_next::op_http_serve_on<HTTP>,
     http_next::op_http_serve<HTTP>,

tests/unit/command_test.ts

@@ -14,27 +14,31 @@ Deno.test(
     const enc = new TextEncoder();
     const cwd = await Deno.makeTempDir({ prefix: "deno_command_test" });
 
+    const exitCodeFileLock = "deno_was_here.lock";
     const exitCodeFile = "deno_was_here";
     const programFile = "poll_exit.ts";
     const program = `
+const file = await Deno.open("${exitCodeFileLock}", { write: true, create: true });
 async function tryExit() {
+  await file.lock(true);
   try {
     const code = parseInt(await Deno.readTextFile("${exitCodeFile}"));
     Deno.exit(code);
   } catch {
     // Retry if we got here before deno wrote the file.
     setTimeout(tryExit, 0.01);
+  } finally {
+    await file.unlock();
   }
 }
 
 tryExit();
 `;
-
     Deno.writeFileSync(`${cwd}/${programFile}`, enc.encode(program));
 
     const command = new Deno.Command(Deno.execPath(), {
       cwd,
-      args: ["run", "--allow-read", programFile],
+      args: ["run", "-RW", programFile],
       stdout: "inherit",
       stderr: "inherit",
     });
@@ -43,12 +47,18 @@ tryExit();
     // Write the expected exit code *after* starting deno.
     // This is how we verify that `Child` is actually asynchronous.
     const code = 84;
-    Deno.writeFileSync(`${cwd}/${exitCodeFile}`, enc.encode(`${code}`));
 
+    await using file = await Deno.open(`${cwd}/${exitCodeFileLock}`, {
+      write: true,
+      create: true,
+    });
+    await file.lock(true);
+    Deno.writeFileSync(`${cwd}/${exitCodeFile}`, enc.encode(`${code}`));
+    await file.unlock();
     const status = await child.status;
     await Deno.remove(cwd, { recursive: true });
-    assertEquals(status.success, false);
     assertEquals(status.code, code);
+    assertEquals(status.success, false);
     assertEquals(status.signal, null);
   },
 );

tests/unit/serve_test.ts

@@ -4270,3 +4270,32 @@ Deno.test({
   assertEquals(hostname, "0.0.0.0");
   await server.shutdown();
 });
+
+Deno.test({
+  name: "AbortSignal aborted when request is cancelled",
+}, async () => {
+  const { promise, resolve } = Promise.withResolvers<void>();
+
+  let cancelled = false;
+
+  const server = Deno.serve({
+    hostname: "0.0.0.0",
+    port: servePort,
+    onListen: () => resolve(),
+  }, async (request) => {
+    request.signal.addEventListener("abort", () => cancelled = true);
+    assert(!request.signal.aborted);
+    await new Promise((resolve) => setTimeout(resolve, 3000)); // abort during waiting
+    assert(request.signal.aborted);
+    return new Response("Ok");
+  });
+
+  await promise;
+  await fetch(`http://localhost:${servePort}/`, {
+    signal: AbortSignal.timeout(1000),
+  }).catch(() => {});
+
+  await server.shutdown();
+
+  assert(cancelled);
+});