Resource leak when (re-)using an abort signal

[marcelmeulemans]

Describe the bug

• Node.js version: v16.17.1
• OS & version: Debian 5.10.140-1

When re-using an abort controller for multiple got requests a significant amount of memory is retained by the AbortSignal through got. The main reason for this (I think) is that the Request instance adds an event listener for the abortevent on the passed signal where the event handler refers back to the request. The event handler is never removed so the signal retains a reference to the Request instance for as long as it lives.

Actual behavior

The code to reproduce below downloads and discards a 1MB file in a loop. The loop exists when the abort signal is aborted via ctrl-c. The same abort signal is passed to got to cancel the in-progress download. The heap profile screenshot below shows how a large chunk of memory is retained via the callback context of an event listener coming from here:
https://github.com/sindresorhus/got/blob/main/source/core/index.ts#L254

Expected behavior

Memory is released after the request completes.

Code to reproduce

import got from 'got';

const abortController = new AbortController();

process.on('SIGINT', function () {
  abortController.abort();
});

async function main() {
  while (!abortController.signal.aborted) {
    await got.get(
      'https://ia902805.us.archive.org/14/items/1mbFile/1mb.mp4',
      {
        signal: abortController.signal,
      }
    ).buffer();
  }
}

main().then(() => console.log('Bye')).catch((e) => {
  if (e.constructor.name !== 'AbortError') {
    console.error(e);
  }
});

Checklist

• [*] I have read the documentation.
• [*] I have tried my code with the latest version of Node.js and Got.

[sindresorhus]

// @jopemachine

[szmarczak]

got/source/core/index.ts

Lines 254 to 256 in 623229f

	this.options.signal?.addEventListener('abort', () => {
	this.destroy(new AbortError(this));
	});

this should be

		this.options.signal?.addEventListener('abort', () => {
			this.destroy(new AbortError(this));
		}, { once: true });

additionally, we should do

this.options.signal?.removeEventListener('abort', abortListener);

somewhere in _destroy.