From 67e8f47b36f39df56d962aafb40728728673c109 Mon Sep 17 00:00:00 2001 From: Orgad Shaneh Date: Tue, 31 Mar 2026 23:17:55 +0300 Subject: [PATCH] dns: support AbortSignal in dns.lookup() and dns.promises.lookup() Add a `signal` option to both `dns.lookup()` and `dns.promises.lookup()` that accepts an AbortSignal, allowing callers to cancel pending lookups. When `getaddrinfo` blocks a libuv thread for an extended period (e.g. ~10s on EAI_AGAIN), new lookups queue behind the blocked threads and the number of unresolved promises grows without bound. Each pending promise holds references to the calling closure, preventing GC and causing a memory leak in long-running servers under sustained DNS failure. With this change, callers can abort stale lookups: const ac = new AbortController(); setTimeout(() => ac.abort(), 5000); await dns.promises.lookup(host, { signal: ac.signal }); If the signal is already aborted, the call rejects/calls back immediately without dispatching to libuv. If aborted while pending, the promise is rejected (or callback called) with an AbortError and the late oncomplete from libuv is silently ignored. Fixes: https://github.com/nodejs/node/issues/62503 PR-URL: https://github.com/nodejs/node/pull/62528 --- doc/api/dns.md | 6 + lib/dns.js | 33 ++++ lib/internal/dns/promises.js | 60 ++++-- test/parallel/test-dns-lookup-abort-signal.js | 172 ++++++++++++++++++ 4 files changed, 259 insertions(+), 12 deletions(-) create mode 100644 test/parallel/test-dns-lookup-abort-signal.js diff --git a/doc/api/dns.md b/doc/api/dns.md index b2e350bfc6caa6..2f7a98e69c7ae4 100644 --- a/doc/api/dns.md +++ b/doc/api/dns.md @@ -274,6 +274,9 @@ changes: **Default:** `true` (addresses are not reordered). Default value is configurable using [`dns.setDefaultResultOrder()`][] or [`--dns-result-order`][]. + * `signal` {AbortSignal} An AbortSignal that may be used to cancel an + in-progress lookup. If the signal is aborted, the callback is called with + an `AbortError`. * `callback` {Function} * `err` {Error} * `address` {string} A string representation of an IPv4 or IPv6 address. @@ -1125,6 +1128,9 @@ changes: expected to change in the not too distant future. Default value is configurable using [`dns.setDefaultResultOrder()`][] or [`--dns-result-order`][]. + * `signal` {AbortSignal} An AbortSignal that may be used to cancel an + in-progress lookup. If the signal is aborted, the returned `Promise` is + rejected with an `AbortError`. Resolves a host name (e.g. `'nodejs.org'`) into the first found A (IPv4) or AAAA (IPv6) record. All `option` properties are optional. If `options` is an diff --git a/lib/dns.js b/lib/dns.js index d651f5ea0a2685..befa27ca38e645 100644 --- a/lib/dns.js +++ b/lib/dns.js @@ -31,6 +31,7 @@ const cares = internalBinding('cares_wrap'); const { isIP } = require('internal/net'); const { customPromisifyArgs } = require('internal/util'); const { + AbortError, DNSException, codes: { ERR_INVALID_ARG_TYPE, @@ -78,6 +79,7 @@ const { CANCELLED, } = dnsErrorCodes; const { + validateAbortSignal, validateBoolean, validateFunction, validateNumber, @@ -86,6 +88,8 @@ const { validateString, } = require('internal/validators'); +let kResistStopPropagation; + const { GetAddrInfoReqWrap, GetNameInfoReqWrap, @@ -106,6 +110,7 @@ const { let promises = null; // Lazy loaded function onlookup(err, addresses) { + if (this.aborted) return; if (err) { return this.callback(new DNSException(err, 'getaddrinfo', this.hostname)); } @@ -117,6 +122,7 @@ function onlookup(err, addresses) { function onlookupall(err, addresses) { + if (this.aborted) return; if (err) { return this.callback(new DNSException(err, 'getaddrinfo', this.hostname)); } @@ -143,6 +149,7 @@ function lookup(hostname, options, callback) { let hints = 0; let family = 0; let all = false; + let signal; let dnsOrder = getDefaultResultOrder(); // Parse arguments @@ -195,6 +202,15 @@ function lookup(hostname, options, callback) { validateOneOf(options.order, 'options.order', validDnsOrders); dnsOrder = options.order; } + if (options?.signal != null) { + validateAbortSignal(options.signal, 'options.signal'); + } + signal = options?.signal; + } + + if (signal?.aborted) { + process.nextTick(callback, new AbortError(undefined, { cause: signal.reason })); + return {}; } if (!hostname) { @@ -234,6 +250,23 @@ function lookup(hostname, options, callback) { process.nextTick(callback, new DNSException(err, 'getaddrinfo', hostname)); return {}; } + if (signal) { + const onAbort = () => { + req.aborted = true; + req.callback(new AbortError(undefined, { cause: signal.reason })); + }; + kResistStopPropagation ??= require('internal/event_target').kResistStopPropagation; + signal.addEventListener('abort', onAbort, { + __proto__: null, + once: true, + [kResistStopPropagation]: true, + }); + const originalOncomplete = req.oncomplete; + req.oncomplete = function(...args) { + signal.removeEventListener('abort', onAbort); + return originalOncomplete.apply(this, args); + }; + } if (hasObserver('dns')) { const detail = { hostname, diff --git a/lib/internal/dns/promises.js b/lib/internal/dns/promises.js index f7ee8fd25423ca..6f3d92ccac0a75 100644 --- a/lib/internal/dns/promises.js +++ b/lib/internal/dns/promises.js @@ -4,6 +4,7 @@ const { FunctionPrototypeCall, ObjectDefineProperty, Promise, + SafePromisePrototypeFinally, Symbol, } = primordials; @@ -46,6 +47,7 @@ const { CANCELLED, } = dnsErrorCodes; const { + AbortError, DNSException, codes: { ERR_INVALID_ARG_TYPE, @@ -65,6 +67,7 @@ const { DNS_ORDER_IPV6_FIRST, } = internalBinding('cares_wrap'); const { + validateAbortSignal, validateBoolean, validateNumber, validateOneOf, @@ -72,6 +75,8 @@ const { validateString, } = require('internal/validators'); +let kResistStopPropagation; + const kPerfHooksDnsLookupContext = Symbol('kPerfHooksDnsLookupContext'); const kPerfHooksDnsLookupServiceContext = Symbol('kPerfHooksDnsLookupServiceContext'); const kPerfHooksDnsLookupResolveContext = Symbol('kPerfHooksDnsLookupResolveContext'); @@ -83,6 +88,7 @@ const { } = require('internal/perf/observe'); function onlookup(err, addresses) { + if (this.aborted) return; if (err) { this.reject(new DNSException(err, 'getaddrinfo', this.hostname)); return; @@ -96,6 +102,7 @@ function onlookup(err, addresses) { } function onlookupall(err, addresses) { + if (this.aborted) return; if (err) { this.reject(new DNSException(err, 'getaddrinfo', this.hostname)); return; @@ -131,8 +138,13 @@ function onlookupall(err, addresses) { * @property {string} address - The IP address. * @property {0 | 4 | 6} family - The IP address type. 4 for IPv4 or 6 for IPv6, or 0 (for both). */ -function createLookupPromise(family, hostname, all, hints, dnsOrder) { - return new Promise((resolve, reject) => { +function createLookupPromise(family, hostname, all, hints, dnsOrder, signal) { + if (signal?.aborted) { + return Promise.reject(new AbortError(undefined, { cause: signal.reason })); + } + + let onabort; + const promise = new Promise((resolve, reject) => { if (!hostname) { reject(new ERR_INVALID_ARG_VALUE('hostname', hostname, 'must be a non-empty string')); @@ -167,17 +179,36 @@ function createLookupPromise(family, hostname, all, hints, dnsOrder) { if (err) { reject(new DNSException(err, 'getaddrinfo', hostname)); - } else if (hasObserver('dns')) { - const detail = { - hostname, - family, - hints, - verbatim: order === DNS_ORDER_VERBATIM, - order: dnsOrder, - }; - startPerf(req, kPerfHooksDnsLookupContext, { type: 'dns', name: 'lookup', detail }); + } else { + if (signal) { + onabort = () => { + req.aborted = true; + reject(new AbortError(undefined, { cause: signal.reason })); + }; + kResistStopPropagation ??= require('internal/event_target').kResistStopPropagation; + signal.addEventListener('abort', onabort, { + __proto__: null, + once: true, + [kResistStopPropagation]: true, + }); + } + if (hasObserver('dns')) { + const detail = { + hostname, + family, + hints, + verbatim: order === DNS_ORDER_VERBATIM, + order: dnsOrder, + }; + startPerf(req, kPerfHooksDnsLookupContext, { type: 'dns', name: 'lookup', detail }); + } } }); + + return onabort !== undefined ? + SafePromisePrototypeFinally( + promise, + () => signal.removeEventListener('abort', onabort)) : promise; } /** @@ -196,6 +227,7 @@ function lookup(hostname, options) { let hints = 0; let family = 0; let all = false; + let signal; let dnsOrder = getDefaultResultOrder(); // Parse arguments @@ -230,9 +262,13 @@ function lookup(hostname, options) { validateOneOf(options.order, 'options.order', validDnsOrders); dnsOrder = options.order; } + if (options?.signal != null) { + validateAbortSignal(options.signal, 'options.signal'); + } + signal = options?.signal; } - return createLookupPromise(family, hostname, all, hints, dnsOrder); + return createLookupPromise(family, hostname, all, hints, dnsOrder, signal); } diff --git a/test/parallel/test-dns-lookup-abort-signal.js b/test/parallel/test-dns-lookup-abort-signal.js new file mode 100644 index 00000000000000..517f817cdeb47b --- /dev/null +++ b/test/parallel/test-dns-lookup-abort-signal.js @@ -0,0 +1,172 @@ +// Flags: --expose-internals +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const { internalBinding } = require('internal/test/binding'); +const cares = internalBinding('cares_wrap'); + +// Stub `getaddrinfo` to proxy its call to a dynamic stub. This has to be done +// before we load the `dns` module to guarantee that the `dns` module uses the +// stub. +let getaddrinfoStub = null; +cares.getaddrinfo = (req) => getaddrinfoStub(req); + +const dns = require('dns'); +const dnsPromises = dns.promises; + +// dns.promises.lookup — already-aborted signal rejects immediately without +// calling getaddrinfo +async function promisesLookupAlreadyAborted() { + getaddrinfoStub = common.mustNotCall( + 'getaddrinfo must not be called when signal is already aborted', + ); + const ac = new AbortController(); + ac.abort('my reason'); + await assert.rejects( + dnsPromises.lookup('example.com', { signal: ac.signal }), + (err) => { + assert.strictEqual(err.name, 'AbortError'); + assert.strictEqual(err.cause, 'my reason'); + return true; + }, + ); +} + +// dns.promises.lookup — abort during pending request rejects, and the late +// oncomplete is silently ignored (no double-settle) +async function promisesLookupAbortDuringPending() { + const ac = new AbortController(); + let savedReq; + getaddrinfoStub = common.mustCall((req) => { + savedReq = req; + return 0; + }); + const promise = dnsPromises.lookup('example.com', { signal: ac.signal }); + ac.abort('cancelled'); + await assert.rejects( + promise, + (err) => { + assert.strictEqual(err.name, 'AbortError'); + assert.strictEqual(err.cause, 'cancelled'); + return true; + }, + ); + // Simulate the late oncomplete arriving after abort — must not throw or + // cause an unhandled rejection. + savedReq.oncomplete(null, ['127.0.0.1']); +} + +// dns.promises.lookup — signal not aborted, request completes normally and +// the abort listener is removed from the signal +async function promisesLookupNormalCompletion() { + const ac = new AbortController(); + getaddrinfoStub = common.mustCall((req) => { + req.oncomplete(null, ['127.0.0.1']); + return 0; + }); + const result = await dnsPromises.lookup('example.com', { signal: ac.signal }); + assert.deepStrictEqual(result, { address: '127.0.0.1', family: 4 }); + // The abort listener must have been cleaned up — aborting now must not + // trigger any side-effects. + ac.abort(); +} + +// dns.lookup (callback) — already-aborted signal calls back with AbortError +// without calling getaddrinfo +async function callbackLookupAlreadyAborted() { + getaddrinfoStub = common.mustNotCall( + 'getaddrinfo must not be called when signal is already aborted', + ); + const ac = new AbortController(); + ac.abort('my reason'); + return new Promise((resolve) => { + dns.lookup('example.com', { signal: ac.signal }, common.mustCall((err) => { + assert.strictEqual(err.name, 'AbortError'); + assert.strictEqual(err.cause, 'my reason'); + resolve(); + })); + }); +} + +// dns.lookup (callback) — abort during pending request calls back with +// AbortError, and late oncomplete is silently ignored +async function callbackLookupAbortDuringPending() { + const ac = new AbortController(); + let savedReq; + getaddrinfoStub = common.mustCall((req) => { + savedReq = req; + return 0; + }); + return new Promise((resolve) => { + dns.lookup('example.com', { signal: ac.signal }, + common.mustCall((err) => { + assert.strictEqual(err.name, 'AbortError'); + assert.strictEqual(err.cause, 'cancelled'); + // Simulate late oncomplete — must not call the callback again + // (mustCall already asserts exactly-once). + savedReq.oncomplete(null, ['127.0.0.1']); + resolve(); + })); + ac.abort('cancelled'); + }); +} + +// dns.lookup (callback) — signal not aborted, completes normally and the +// abort listener is removed +async function callbackLookupNormalCompletion() { + const ac = new AbortController(); + getaddrinfoStub = common.mustCall((req) => { + req.oncomplete(null, ['127.0.0.1']); + return 0; + }); + return new Promise((resolve) => { + dns.lookup('example.com', { signal: ac.signal }, + common.mustCall((err, address, family) => { + assert.ifError(err); + assert.strictEqual(address, '127.0.0.1'); + assert.strictEqual(family, 4); + // Aborting after completion must not trigger any side-effects. + ac.abort(); + resolve(); + })); + }); +} + +// dns.promises.lookup — without signal, lookup still works +async function promisesLookupWithoutSignal() { + getaddrinfoStub = common.mustCall((req) => { + req.oncomplete(null, ['127.0.0.1']); + return 0; + }); + const result = await dnsPromises.lookup('example.com'); + assert.deepStrictEqual(result, { address: '127.0.0.1', family: 4 }); +} + +// dns.promises.lookup — invalid signal type throws +async function promisesLookupInvalidSignal() { + assert.throws( + () => dnsPromises.lookup('example.com', { signal: 'not-a-signal' }), + { code: 'ERR_INVALID_ARG_TYPE' }, + ); +} + +// dns.lookup — invalid signal type throws +async function callbackLookupInvalidSignal() { + assert.throws( + () => dns.lookup('example.com', { signal: 'not-a-signal' }, + common.mustNotCall()), + { code: 'ERR_INVALID_ARG_TYPE' }, + ); +} + +(async () => { + await promisesLookupAlreadyAborted(); + await promisesLookupAbortDuringPending(); + await promisesLookupNormalCompletion(); + await callbackLookupAlreadyAborted(); + await callbackLookupAbortDuringPending(); + await callbackLookupNormalCompletion(); + await promisesLookupWithoutSignal(); + await promisesLookupInvalidSignal(); + await callbackLookupInvalidSignal(); +})().then(common.mustCall());