Aggressive memory leak in Repeater.race

[brainkim]

Reported by @elderapo. When Repeater.race is called with an iterator and a promise which doesn’t settle, memory usage grows until the process crashes.

Reproduction:

import * as crypto from "crypto";
import {Repeater} from "@repeaterjs/repeater";
async function randomString(): Promise<string> {
  await new Promise((resolve) => setTimeout(resolve, 1));
  return crypto.randomBytes(10000).toString("hex")
}

async function *createStrings() {
  while (true) {
    yield randomString();
  }
}

function map<T, U>(iter: AsyncIterable<T>, fn: (value: T) => U): AsyncGenerator<U> {
  return new Repeater(async (push, stop) => {
    for await (const value of Repeater.race([iter, stop])) {
      push(fn(value as T) as U);
    }

    stop();
  });
}

(async function main() {
  let i = 0;
  for await (const value of map(createStrings(), (value) => value.split(""))) {
    if (i++ % 1000 === 0) {
      const usage = process.memoryUsage();
      const rssMiB = Math.round(usage.rss / (1024 ** 2) * 100) / 100;
      console.log(`Resident Set: ${rssMiB} MiB`);
    }
  }
})();

This will log an increasing resident set size until the process runs out of memory. It seems to be exacerbated by what is passed through the iterator which is racing the stop promise.

[brainkim]

Initial investigation shows that calling Promise.race with an iteration and the stop promise (or any promise which does not settle) spawns a promise reaction for the stop promise, and this reaction retains the paired iteration. This is a consequence of the implementation of Promise.race, which internally is implemented by calling then on each of its contenders. What seems to be happening is that even if a promise loses, the reaction which was created by Promise.race is retained until the original promise settles, and that reaction was created in the context of the other promise.

Part of me wonders if Repeater.race is fundamentally flawed, another part of me is frustrated to come across yet another promise weirdness. More investigation needed.

Relevant issues: https://github.com/nodejs/node/issues/17469 https://bugs.chromium.org/p/v8/issues/detail?id=9858

[brainkim]

Here’s a reproduction of the leak without repeaters:

async function randomString(): Promise<string> {
  await new Promise((resolve) => setTimeout(resolve, 1));
  return crypto.randomBytes(10000).toString("hex")
}

async function *createStrings() {
  while (true) {
    yield randomString();
  }
}

(async function main() {
  let i = 0;
  let iteration: IteratorResult<string>;
  const iterator = createStrings();
  const pending: Promise<never> = new Promise(() => {});
  do {
    iteration = await Promise.race([pending, iterator.next()]);
    if (i++ % 1000 === 0) {
      const usage = process.memoryUsage();
      const rssMiB = Math.round(usage.rss / (1024 ** 2) * 100) / 100;
      console.log(`Resident Set: ${rssMiB} MiB`);
    }
  } while (!iteration.done);
})();