Possible memory leak?

[Codex-]

Hi, i'm finding when running the queue for extended periods there appear to be handles held / memory not reducing over time.

i.e.:

import PQueue from 'p-queue';

class Cow {
  beef: number;

  constructor(i: number) {
    this.beef = i;
  }
}

async function test(queue: any) {
  for (let j = 0; j < 30000; j++) {
    const promises = [];
    console.log(`Start... ${j}`);

    for (let i = 0; i < 1000; i++) {
      promises.push(
        queue.add(async () => {
          const arr = [];
          for (let i = 0; i < 100; i++) {
            arr.push(new Cow(i));
          }
        }),
      );
    }

    await Promise.all(promises);
  }
}

(async () => {
  const queue = new PQueue({ concurrency: 5 });
  await test(queue);
  console.log('Completed');
  while (true) {}
})();

When executing this locally with node 10 the memory used before populating the queue is ~8mb but after executing this sits at around ~78mb with the cause of this simple object.

I would have expected the function completion to mark the items in the functions closure as safe for GC.

In my bigger use case, there are objects being delete from a queue elsewhere but they're stuck in memory when dumping the memory.

Am I using the queue incorrectly? Should I be calling a clear of some kind bound to a completion event?

[Richienb]

The queue variable never goes out of scope so the garbage collector never removes it.

A better experiment would also manually invoke the garbage collector since it tends to be run whenever the engine wants it to. Try this with node --expose-gc:

import PQueue from 'p-queue';

class Cow {
	constructor(i) {
		this.beef = i;
	}
}

async function test(queue) {
	for (let j = 0; j < 30_000; j++) {
		if (j % 1000 === 0) {
			console.log("Running:", j);
		}

		const promises = [];

		for (let i = 0; i < 1000; i++) {
			promises.push(
				queue.add(async () => {
					const array = [];
					for (let i = 0; i < 100; i++) {
						array.push(new Cow(i));
					}
				}),
			);
		}

		await Promise.all(promises);
	}
}

console.log("Before running:", process.memoryUsage());

await (async () => {
	const queue = new PQueue({ concurrency: 5 });
	await test(queue);
	console.log('Completed');
})();

console.log("After running:", process.memoryUsage());

gc();

console.log("After garbage collection:", process.memoryUsage());

Output:

Before running: {
  rss: 27095040,
  heapTotal: 4915200,
  heapUsed: 4509376,
  external: 503134,
  arrayBuffers: 70350
}
...
After running: {
  rss: 62517248,
  heapTotal: 41623552,
  heapUsed: 20917184,
  external: 469655,
  arrayBuffers: 11166
}
After garbage collection: {
  rss: 61730816,
  heapTotal: 40574976,
  heapUsed: 4136168,
  external: 468417,
  arrayBuffers: 10430
}

Notice how heapUsed is 16 megabytes smaller after garbage collection then after running, somehow dropping even lower than before running.

[Codex-]

This doesn't really address the problem though, for an item that has been dequeued and no longer in the scope of the queue it should be dereferenced and gc'd, but in this case, it wasn't (I haven't retested since 3 years ago and ended up writing one for the use case lol)

Long-lived queues shouldn't need to be deleted and re-instantiated for their long dequeued items to be collected, seems like very undesirable behaviour.

For example, consider the case where a queue instance is used in a convenience class that provides some functionality that consumes items and queues them, then based on some criteria dequeues. At the end of the queue, it seems undesireable to then manage creating a new instance of the queue each time?

[Richienb]

The original example intentionally holds on to every single queued item in an array and passes them all to Promise.all. With every reference in use, no garbage collection can happen.