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

abortableAsyncIterable() should call .return() on the generator once aborted #3518

Closed
chromakode opened this issue Aug 6, 2023 · 3 comments · Fixed by #5560
Closed

abortableAsyncIterable() should call .return() on the generator once aborted #3518

chromakode opened this issue Aug 6, 2023 · 3 comments · Fixed by #5560
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@chromakode
Copy link

Describe the bug

Async generators canceled by abortableAsyncIterable don't have their return method called, so they don't get the opportunity to clean up resources.

https://github.com/denoland/deno_std/blob/d84f4b1c947b6b09df5e9a2e50e8f1549fe615fd/async/abortable.ts#L135-L139

Expected behavior

An example of a library which handles this is abortable-iterator:

https://github.com/alanshaw/abortable-iterator/blob/68c554cb40bd160216abb7200b54015aad0bd9a8/src/index.ts#L103-L106

Environment

  • std version: 1.36.0
@chromakode chromakode added bug Something isn't working needs triage labels Aug 6, 2023
@iuioiua
Copy link
Contributor

iuioiua commented Nov 23, 2023

Contributions are welcome 🙂

@iuioiua iuioiua added good first issue Good for newcomers and removed needs triage labels Nov 23, 2023
@iuioiua iuioiua changed the title abortableAsyncIterable should call return on the generator once aborted abortableAsyncIterable() should call .return() on the generator once aborted Dec 30, 2023
@raaymax
Copy link
Contributor

raaymax commented Jul 27, 2024

const stream = source
      .pipeThrough(new TextDecoderStream())
      .pipeThrough(new TextLineStream({ allowCR: true }))

try {
  for await (const message of abortable(stream, signal)) {
    this.dispatch({ message });
  }
} catch (e) {
  console.error(e)
} 

this results with:

error: Leaks detected:
  - A text decoder was created during the test, but not finished during the test. Close the text decoder by calling `textDecoder.decode('')` or `await textDecoderStream.readable.cancel()`.

and when trying to stream.cancel() in catch I get this:

TypeError: Cannot cancel a locked ReadableStream.

@raaymax
Copy link
Contributor

raaymax commented Aug 1, 2024

@iuioiua Interestingly this works when stream is created by fetch for example but I'm having strange errors when using streams created in code but this is a problem with deno itself I think. Reported it here:
denoland/deno#19946 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants