recommend avoiding async generators, they can easily leak resources

[jedwards1211]

WHATWG web streams have the important property that as soon as the consumer signals they are done by canceling the reader, it promptly communicates that to everything upstream so that everything can be cleaned up right away.

Async generators don't work as well, unfortunately. Even if the consumer .return()s its async iterator as soon as it's done, it may take a long time for the source iterator to find out: if you're passing the stream through an async generator, the async generator has to wait for the next chunk to come through before it can exit its loop and .return() the source iterator.

Well, if the source iterator is wrapping an event stream like a Redis topic, that next event may never come. In this case the async generator is stuck permanently, causing a resource leak, unless the source is aborted via a signal or designed to time out. I dealt with leaks like this in the early days of graphql subscriptions when the libraries didn't support abort signals. I had to convert all my filtering and mapping to avoid async generators to fix the leaks.

I was in love with async iteration at first but I became very disillusioned after this. This is a major weakness in the design, and async iteration no longer feels comfy and nice to me, it feels dangerous.

It's good that you support passing an abort signal to the source stream, but there's no guarantee users will structure things in a way that allows them to abort the source stream when they're done iterating something downstream, so there's a good chance some people wrapping event streams will accidentally suffer resource leaks like this.

So I would recommend warning about this very, very clearly in the documentation. These leaks are completely unexpected, hard to debug, and hard to wrap your head around.

The advice unfortunately needs to be: avoid async generators unless you know the source stream will error or time out if it never receives another chunk, and you're okay with waiting that long to clean up, even if the consumer is already done.

Btw, async iterating WHATWG streams has this same problem. Only canceling them is fully safe.

[robbiespeed]

Given that iterators are a single consumer object is it not expected that the code which consumes it also owns production and cancellation?

[jedwards1211]

@robbiespeed no. You may get the producing async iterator from a library (as I was from graphql-subscriptions), pass it through an async generator transform, and then return that to an IoC system (e.g. graphql resolution) that's responsible for cancelation.

In my case the old version of graphql would just .return() the iterator I gave it when the client unsubscribed, which is insufficient to immediately abort an upstream async generator. Hopefully they pass an abort signal to the subscription resolver now, and hopefully graphql-subscriptions accepts an abort signal now.

[jasnell]

I think the approach here is to explore a more active cancelation path. For instance, possibly passing an AbortSignal down to generators has been suggested. TC-39 will also be having a discussion again about the defining a language level abort protocol ... that said, if there are specific improvements to the iterable protocol to make cancelation more immediate, we should discuss those also. I don't, however, believe that this alone is enough of a reason to fully avoid the async generator model.

[jedwards1211]

@jasnell I'm surprised your reaction wasn't at least something like "oh gosh, this is subtle and surprising, I do really want to warn users about this to make sure the have the best experience they can with my tool."

I guess you don't viscerally understand how painful running into this situation can be. It was spread across days of frustrated debugging and googling for me, making stabs in the dark to try to fix the issue and still seeing my web server tasks keep OOMing until I finally figured it out. I'm not sure how else to convey it but I think if you had my experience you wouldn't be so unconcerned.

Right now, for anyone who decides to use your design for event streams (and someone will), the trap is set and quite a lot of suffering is in store for them when they run into this issue - unless you can manage to warn them before they make this mistake.

You want people to have the most pleasant experience they can with your API design, yes? I am trying to help you, and help your users, by bringing this up. Let's add some warnings to the docs if nothing else.

Because right now if people follow examples of using async generators from your repo, but happen to apply it to event streams, they'll get OOM issues.

[midnight-wonderer]

Hey, I'm just a passerby.
I'm wondering how wrapping event streams into async generators could cause a leak.
Are we talking about something like Repeater.js?

---

I recently learned about Repeater.js from Hacker News, but I have never used it myself.
Below is some context (generated by AI) introducing the library.

---

Recap what already in the thread

The "leak" happens because async generators are pull-based. When a consumer stops iterating (e.g., via break), the engine calls .return() on the iterator. However, an async generator that is currently await-ing its next source chunk is suspended. It won't actually process that .return() signal until it resumes.

If the source is an event stream (like a Redis topic) that never sends another event, the generator stays suspended in memory forever. It effectively leaks its entire scope and any resources it's holding onto, because it's waiting for an event that will never come to tell it that it's time to die.

How Repeater.js fixes this:

Repeater.js introduces a push-based side to the iterator. It provides a stop promise that resolves the moment the consumer stops. This allows you to race your event-waiting against the stop signal:

const repeater = new Repeater(async (push, stop) => {
  while (true) {
    // If the consumer stops, 'stop' resolves immediately, 
    // breaking the wait even if no event arrived.
    const event = await Promise.race([getNextEvent(), stop]);
    if (event === undefined) break; 
    await push(event);
  }
});

Without a mechanism like stop or a passed-in AbortSignal, async generators are essentially "zombies" when they go idle—they are dead to the consumer, but they won't stop existing until they get one last nudge from the producer.

FWIW, callback-based wrapping could look like this:

const repeater = new Repeater(async (push, stop) => {
  emitter.on('data', push);
  await stop;
  emitter.off('data', push);
});

[jedwards1211]

@midnight-wonderer in many ways the AI is correct but it's wrong about one thing: that Promise.race will necessarily fix the leak. Welcome to another nasty surprise of JS concurrency: Promise.race can also leak memory when one of the raced promises never resolves.

Basically even if the race resolves because of stop, as long as getNextEvent() is outstanding, it causes any handlers registered on Promise.race to be retained (even after the race is over). Unfortunately this is what the spec requires.

Whether that actually causes a leak in this example depends on what Repeater.js does after the producer method ends. Repeater.js may contain this leak, but only if it intentionally breaks the references between the promises from the producer, and the handlers registered by the consumer, after iteration is done, so that the consumer handlers are no longer retained.

Also, Repeater.js won't protect any better than any other method of creating an async iterable against leaks from transforming it through an async generator. .return()ing an async iterator when the consumer is done just isn't sufficient to clean up subscriptions if there's an async generator in the chain.

[jedwards1211]

@midnight-wonderer here's a long thread about how wrapping event streams (or any async iterable of events, really) into async generators can cause leaks.

[jedwards1211]

@jasnell

I think the approach here is to explore a more active cancelation path. For instance, nodejs/node#62066 (comment) has been suggested.

We aren't lacking an active cancelation path from the underlying source end or from transformations.

It's a more active cancelation path from the consumer end that's missing here. (And I'm talking about a consumer that just receives an async iterable object it doesn't get to control the construction of). A consumer doesn't have a reliable cancelation path via an async iterable or async iterator object by itself. It currently requires something outside the async iteration protocol like an AbortController or stream object with a cancel method.

A stream implementation is only good for coordination between disparate parts of a system if the consumer is able to iterate and cancel via the same object.

[midnight-wonderer]

It feels like I'm falling down a rabbit hole.

Interestingly, the most popular comment in the thread you shared is from the creator of Repeater.js. The library includes Repeater.race, which still uses Promise.race under the hood; however, since the author is aware of the issue, I'd assume his implementation is safe.

Note: For those interested, the Repeater.js documentation is worth exploring. While the project's presentation is modest, the library itself seems battle-tested.

Regardless, the Promise.race leak is a significant pitfall. There's a certain irony in using it to prevent memory leaks, only for the mechanism itself to become the source of one.

---

I am glad you brought this up. I imagine the final API, while somewhat async generators-ish, probably won't be able to abstract all details away and will likely need some caveats.

[jedwards1211]

I've been to hell and back dealing with the memory leak pitfalls of async JS

[jedwards1211]

Everyone is in love with how nice and clean for..await and async generator syntax is, but intensive async work with event-based systems tends to require ugly workarounds to be safe for production. I made these packages to deal with leaks I've encountered:

https://github.com/jcoreio/abortable
https://github.com/jcoreio/safe-iterate-readable-stream