Skip to content

Commit

Permalink
util: allow safely adding listener to abortSignal
Browse files Browse the repository at this point in the history
  • Loading branch information
atlowChemi committed Jun 29, 2023
1 parent e8810b9 commit d8c8109
Show file tree
Hide file tree
Showing 3 changed files with 145 additions and 0 deletions.
59 changes: 59 additions & 0 deletions doc/api/util.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,64 @@ it:
const util = require('node:util');
```

## `util.addAbortListener(signal, resource)`

<!-- YAML
added: REPLACEME
-->

> Stability: 1 - Experimental
* `signal` {AbortSignal}
* `listener` {Function|EventListener}
* Returns: {Disposable} that removes the `abort` listener.

Listens once to the `abort` event on the provided `signal`.

Listening to the `abort` event on abort signals is unsafe and may
lead to resource leaks since another third party with the signal can
call [`e.stopImmediatePropagation()`][]. Unfortunately Node.js cannot change
this since it would violate the web standard. Additionally, the original
API makes it easy to forget to remove listeners.

This API allows safely using `AbortSignal`s in Node.js APIs by solving these
two issues by listening to the event such that `stopImmediatePropagation` does
not prevent the listener from running.

Returns a disposable so that it may be unsubscribed from more easily.

```cjs
const { addAbortListener } = require('node:util');

function example(signal) {
let disposable;
try {
signal.addEventListener('abort', (e) => e.stopImmediatePropagation());
disposable = addAbortListener(signal, (e) => {
// Do something when signal is aborted.
});
} finally {
disposable?.[Symbol.dispose]();
}
}
```
```mjs
import { addAbortListener } from 'node:util';

function example(signal) {
let disposable;
try {
signal.addEventListener('abort', (e) => e.stopImmediatePropagation());
disposable = addAbortListener(signal, (e) => {
// Do something when signal is aborted.
});
} finally {
disposable?.[Symbol.dispose]();
}
}
```
## `util.callbackify(original)`
<!-- YAML
Expand Down Expand Up @@ -3354,6 +3412,7 @@ util.log('Timestamped message.');
[`WebAssembly.Module`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/WebAssembly/Module
[`assert.deepStrictEqual()`]: assert.md#assertdeepstrictequalactual-expected-message
[`console.error()`]: console.md#consoleerrordata-args
[`e.stopImmediatePropagation()`]: events.md#event.stopImmediatePropagation
[`mime.toString()`]: #mimetostring
[`mimeParams.entries()`]: #mimeparamsentries
[`napi_create_external()`]: n-api.md#napi_create_external
Expand Down
31 changes: 31 additions & 0 deletions lib/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ const {
ObjectValues,
ReflectApply,
StringPrototypePadStart,
SymbolDispose,
} = primordials;

const {
Expand All @@ -63,6 +64,7 @@ const {
} = require('internal/util/inspect');
const { debuglog } = require('internal/util/debuglog');
const {
validateAbortSignal,
validateFunction,
validateNumber,
} = require('internal/validators');
Expand All @@ -77,6 +79,7 @@ const {
toUSVString,
defineLazyProperties,
} = require('internal/util');
const { queueMicrotask } = require('internal/process/task_queues');

let abortController;

Expand All @@ -86,6 +89,7 @@ function lazyAbortController() {
}

let internalDeepEqual;
let kResistStopPropagation;

/**
* @deprecated since v4.0.0
Expand Down Expand Up @@ -345,11 +349,38 @@ function getSystemErrorName(err) {
return internalErrorName(err);
}

function addAbortListener(signal, listener) {
if (signal === undefined) {
throw new ERR_INVALID_ARG_TYPE('signal', 'AbortSignal', signal);
}
validateAbortSignal(signal, 'signal');
validateFunction(listener, 'listener');

let removeEventListener = () => {};
if (signal.aborted) {
queueMicrotask(() => listener());
} else {
kResistStopPropagation ??= require('internal/event_target').kResistStopPropagation;
// TODO(atlowChemi) add { subscription: true } and return directly
signal.addEventListener('abort', listener, { __proto__: null, once: true, [kResistStopPropagation]: true });
removeEventListener = () => {
signal.removeEventListener('abort', listener);
};
}
return {
__proto__: null,
[SymbolDispose]() {
removeEventListener();
},
};
}

// Keep the `exports =` so that various functions can still be monkeypatched
module.exports = {
_errnoException: errnoException,
_exceptionWithHostPort: exceptionWithHostPort,
_extend,
addAbortListener,
callbackify,
debug: debuglog,
debuglog,
Expand Down
55 changes: 55 additions & 0 deletions test/parallel/test-util-add-safe-abort-signal-abort-listener.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
import * as common from '../common/index.mjs';
import * as util from 'node:util';
import * as assert from 'node:assert';
import { describe, it } from 'node:test';

describe('util.addAbortListener', () => {
it('should throw if signal not provided', () => {
assert.throws(() => util.addAbortListener(), { code: 'ERR_INVALID_ARG_TYPE' });
});

it('should throw if provided signal is invalid', () => {
assert.throws(() => util.addAbortListener(undefined), { code: 'ERR_INVALID_ARG_TYPE' });
assert.throws(() => util.addAbortListener(null), { code: 'ERR_INVALID_ARG_TYPE' });
assert.throws(() => util.addAbortListener({}), { code: 'ERR_INVALID_ARG_TYPE' });
});

it('should throw if listener is not a function', () => {
const { signal } = new AbortController();
assert.throws(() => util.addAbortListener(signal), { code: 'ERR_INVALID_ARG_TYPE' });
assert.throws(() => util.addAbortListener(signal, {}), { code: 'ERR_INVALID_ARG_TYPE' });
assert.throws(() => util.addAbortListener(signal, undefined), { code: 'ERR_INVALID_ARG_TYPE' });
});

it('should return a Disposable', () => {
const { signal } = new AbortController();
const disposable = util.addAbortListener(signal, common.mustNotCall());

assert.strictEqual(typeof disposable[Symbol.dispose], 'function');
});

it('should execute the listener immediately for aborted runners', () => {
const disposable = util.addAbortListener(AbortSignal.abort(), common.mustCall());
assert.strictEqual(typeof disposable[Symbol.dispose], 'function');
});

it('should execute the listener even when event propagation stopped', () => {
const controller = new AbortController();
const { signal } = controller;

signal.addEventListener('abort', (e) => e.stopImmediatePropagation());
util.addAbortListener(
signal,
common.mustCall((e) => assert.strictEqual(e.target, signal)),
);

controller.abort();
});

it('should remove event listeners when disposed', () => {
const controller = new AbortController();
const disposable = util.addAbortListener(controller.signal, common.mustNotCall());
disposable[Symbol.dispose]();
controller.abort();
});
});

0 comments on commit d8c8109

Please sign in to comment.