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

Aborting a CompressionStream/DecompressionStream leaks a resource #14212

Closed
Macil opened this issue Apr 5, 2022 · 3 comments · Fixed by #21199
Closed

Aborting a CompressionStream/DecompressionStream leaks a resource #14212

Macil opened this issue Apr 5, 2022 · 3 comments · Fixed by #21199
Labels
bug Something isn't working correctly

Comments

@Macil
Copy link

Macil commented Apr 5, 2022

If inside of a test you call abort() on the writable end of a CompressionStream or DecompressionStream instead of close(), then you get a Deno error about a resource being leaked.

Deno.test(
  "CompressionStream may be aborted",
  // { sanitizeResources: false },
  async () => {
    const compressionStream = new CompressionStream("gzip");

    async function write() {
      const writer = compressionStream.writable.getWriter();
      await writer.ready;
      await writer.write(new Uint8Array([1, 2, 3]));
      try {
        await writer.abort(new Error("aborting"));
      } catch {
        // abort rejects with the error we give it so ignore that
      }
    }

    async function read() {
      try {
        const reader = compressionStream.readable.getReader();
        console.log("bytes received:", (await reader.read()).value?.byteLength);
        console.log("bytes received:", (await reader.read()).value?.byteLength);
      } catch (err) {
        console.log(
          "got error in read() because of abort call in write()",
          err,
        );
      }
    }

    await Promise.all([
      write(),
      read(),
    ]);
  },
);

If sanitizeResources: false is set, then this gives the expected output:

$ deno test fail_test.ts
Check file:///Users/macil/Coding/streaming-zip/fail_test.ts
running 1 test from file:///Users/macil/Coding/streaming-zip/fail_test.ts
test CompressionStream may be aborted ...bytes received: 19

got error in read() because of abort call in write() Error: aborting
    at write (file:///Users/macil/Coding/streaming-zip/fail_test.ts:12:28)
    at async Promise.all (index 0)
    at async file:///Users/macil/Coding/streaming-zip/fail_test.ts:31:5
    at async testStepSanitizer (deno:runtime/js/40_testing.js:444:7)
    at async asyncOpSanitizer (deno:runtime/js/40_testing.js:145:9)
    at async Object.exitSanitizer [as fn] (deno:runtime/js/40_testing.js:427:9)
    at async runTest (deno:runtime/js/40_testing.js:788:7)
    at async Object.runTests (deno:runtime/js/40_testing.js:986:22)

 ok (4ms)

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out (10ms)

But when run normally, this output happens instead:

$ deno test fail_test.ts
Check file:///Users/macil/Coding/streaming-zip/fail_test.ts
running 1 test from file:///Users/macil/Coding/streaming-zip/fail_test.ts
test CompressionStream may be aborted ...bytes received: 19

got error in read() because of abort call in write() Error: aborting
    at write (file:///Users/macil/Coding/streaming-zip/fail_test.ts:12:28)
    at async Promise.all (index 0)
    at async file:///Users/macil/Coding/streaming-zip/fail_test.ts:31:5
    at async testStepSanitizer (deno:runtime/js/40_testing.js:444:7)
    at async asyncOpSanitizer (deno:runtime/js/40_testing.js:145:9)
    at async resourceSanitizer (deno:runtime/js/40_testing.js:370:7)
    at async Object.exitSanitizer [as fn] (deno:runtime/js/40_testing.js:427:9)
    at async runTest (deno:runtime/js/40_testing.js:788:7)
    at async Object.runTests (deno:runtime/js/40_testing.js:986:22)

 FAILED (5ms)

failures:

CompressionStream may be aborted
AssertionError: Test case is leaking 1 resource:

 - A CompressionStream (rid 3) was created during the test, but not closed during the test. Close the compression stream by calling `await stream.writable.close()`.

    at assert (deno:ext/web/00_infra.js:293:13)
    at resourceSanitizer (deno:runtime/js/40_testing.js:409:7)
    at async Object.exitSanitizer [as fn] (deno:runtime/js/40_testing.js:427:9)
    at async runTest (deno:runtime/js/40_testing.js:788:7)
    at async Object.runTests (deno:runtime/js/40_testing.js:986:22)

failures:

	CompressionStream may be aborted

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out (11ms)

error: Test failed

The same issue exists with DecompressionStream too.

The same issue also exists if the CompressionStream's readable end is canceled (instead of the writable end being aborted):

Details about error in canceling
Deno.test(
  "CompressionStream may be aborted",
  // { sanitizeResources: false },
  async () => {
    const compressionStream = new CompressionStream("gzip");

    async function write() {
      const writer = compressionStream.writable.getWriter();
      await writer.ready;
      try {
        await writer.write(new Uint8Array([1, 2, 3]));
      } catch (err) {
        console.log(
          "got error in write() because of cancel call in read()",
          err,
        );
      }
    }

    async function read() {
      try {
        const reader = compressionStream.readable.getReader();
        await reader.cancel(new Error("canceling"));
      } catch {
        // ignore error rethrown by cancel call
      }
    }

    await Promise.all([
      write(),
      read(),
    ]);
  },
);

Expected output:

$ deno test fail_test.ts
Check file:///Users/macil/Coding/streaming-zip/fail_test.ts
running 1 test from file:///Users/macil/Coding/streaming-zip/fail_test.ts
test CompressionStream may be aborted ...got error in write() because of cancel call in read() Error: canceling
    at read (file:///Users/macil/Coding/streaming-zip/fail_test.ts:23:29)
    at file:///Users/macil/Coding/streaming-zip/fail_test.ts:31:7
    at testStepSanitizer (deno:runtime/js/40_testing.js:444:13)
    at asyncOpSanitizer (deno:runtime/js/40_testing.js:145:15)
    at Object.exitSanitizer [as fn] (deno:runtime/js/40_testing.js:427:15)
    at runTest (deno:runtime/js/40_testing.js:788:18)
    at Object.runTests (deno:runtime/js/40_testing.js:986:28)
    at [deno:cli/tools/test.rs:512:6]:1:21

 ok (4ms)

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out (10ms)

Actual output:

$ deno test fail_test.ts
Check file:///Users/macil/Coding/streaming-zip/fail_test.ts
running 1 test from file:///Users/macil/Coding/streaming-zip/fail_test.ts
test CompressionStream may be aborted ...got error in write() because of cancel call in read() Error: canceling
    at read (file:///Users/macil/Coding/streaming-zip/fail_test.ts:23:29)
    at file:///Users/macil/Coding/streaming-zip/fail_test.ts:31:7
    at testStepSanitizer (deno:runtime/js/40_testing.js:444:13)
    at asyncOpSanitizer (deno:runtime/js/40_testing.js:145:15)
    at resourceSanitizer (deno:runtime/js/40_testing.js:370:13)
    at Object.exitSanitizer [as fn] (deno:runtime/js/40_testing.js:427:15)
    at runTest (deno:runtime/js/40_testing.js:788:18)
    at Object.runTests (deno:runtime/js/40_testing.js:986:28)
    at [deno:cli/tools/test.rs:512:6]:1:21

 FAILED (5ms)

failures:

CompressionStream may be aborted
AssertionError: Test case is leaking 1 resource:

 - A CompressionStream (rid 3) was created during the test, but not closed during the test. Close the compression stream by calling `await stream.writable.close()`.

    at assert (deno:ext/web/00_infra.js:293:13)
    at resourceSanitizer (deno:runtime/js/40_testing.js:409:7)
    at async Object.exitSanitizer [as fn] (deno:runtime/js/40_testing.js:427:9)
    at async runTest (deno:runtime/js/40_testing.js:788:7)
    at async Object.runTests (deno:runtime/js/40_testing.js:986:22)

failures:

	CompressionStream may be aborted

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out (10ms)

error: Test failed
@stale
Copy link

stale bot commented Jun 11, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Jun 11, 2022
@stale stale bot closed this as completed Jun 18, 2022
@Macil
Copy link
Author

Macil commented Jun 18, 2022

This issue still exists in Deno v1.23.0.

@ry ry reopened this Jun 18, 2022
@ry ry added the bug Something isn't working correctly label Jun 18, 2022
@stale stale bot removed stale labels Jun 18, 2022
@j50n
Copy link

j50n commented May 28, 2023

I believe this is still a problem. Would it help to give another reproduction scenario? I am seeing it with a TextDecoderStream, so I think it might be a generic error handling bug in streams.

egfx-notifications added a commit to egfx-notifications/deno that referenced this issue Nov 14, 2023
egfx-notifications added a commit to egfx-notifications/deno that referenced this issue Nov 14, 2023
egfx-notifications added a commit to egfx-notifications/deno that referenced this issue Nov 14, 2023
mmastrac pushed a commit to egfx-notifications/deno that referenced this issue Feb 13, 2024
mmastrac pushed a commit that referenced this issue Feb 13, 2024
…cancellation (#21199)

Based on #21074 and #20741 I was looking for further potential use cases
of `TransformStream` `cancel()` method, so here go `CompressionStream`
and `DecompressionStream`.

Fixes #14212
littledivy pushed a commit that referenced this issue Feb 15, 2024
…cancellation (#21199)

Based on #21074 and #20741 I was looking for further potential use cases
of `TransformStream` `cancel()` method, so here go `CompressionStream`
and `DecompressionStream`.

Fixes #14212
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working correctly
Projects
None yet
3 participants