Skip to content

Commit 0163764

Browse files
committed
fix: memory leak introduced in [email protected]
fixes #12
1 parent 42da3ee commit 0163764

File tree

1 file changed

+63
-46
lines changed

1 file changed

+63
-46
lines changed

index.js

Lines changed: 63 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -1248,6 +1248,23 @@ class Tangerine extends dns.promises.Resolver {
12481248
}
12491249
}
12501250

1251+
// #createAbortController and #releaseAbortController manage all AbortController instances created by this resolver
1252+
// - to support cancel() and
1253+
// - to avoid keeping references in this.abortControllers after a query is finished (which would create a memory leak)
1254+
#createAbortController() {
1255+
const abortController = new AbortController();
1256+
this.abortControllers.add(abortController);
1257+
return abortController;
1258+
}
1259+
1260+
#releaseAbortController(abortController) {
1261+
try {
1262+
this.abortControllers.delete(abortController);
1263+
} catch (err) {
1264+
this.options.logger.debug(err);
1265+
}
1266+
}
1267+
12511268
// Cancel all outstanding DNS queries made by this resolver
12521269
// NOTE: callbacks not currently called with ECANCELLED (prob need to alter got options)
12531270
// (instead they are called with "ABORT_ERR"; see ABORT_ERROR_CODES)
@@ -1265,26 +1282,22 @@ class Tangerine extends dns.promises.Resolver {
12651282

12661283
#resolveByType(name, options = {}, parentAbortController) {
12671284
return async (type) => {
1268-
const abortController = new AbortController();
1269-
this.abortControllers.add(abortController);
1270-
abortController.signal.addEventListener(
1271-
'abort',
1272-
() => {
1273-
this.abortControllers.delete(abortController);
1274-
},
1275-
{ once: true }
1276-
);
1277-
parentAbortController.signal.addEventListener(
1278-
'abort',
1279-
() => {
1280-
try {
1281-
abortController.abort('Parent abort controller aborted');
1282-
} catch (err) {
1283-
this.options.logger.debug(err);
1284-
}
1285-
},
1286-
{ once: true }
1287-
);
1285+
const abortController = this.#createAbortController();
1286+
try {
1287+
parentAbortController.signal.addEventListener(
1288+
'abort',
1289+
() => {
1290+
try {
1291+
abortController.abort('Parent abort controller aborted');
1292+
} catch (err) {
1293+
this.options.logger.debug(err);
1294+
}
1295+
},
1296+
{ once: true }
1297+
);
1298+
} finally {
1299+
this.#releaseAbortController(abortController);
1300+
}
12881301
// wrap with try/catch because ENODATA shouldn't cause errors
12891302
try {
12901303
switch (type) {
@@ -1379,6 +1392,8 @@ class Tangerine extends dns.promises.Resolver {
13791392

13801393
if (err.code === dns.NODATA) return;
13811394
throw err;
1395+
} finally {
1396+
this.#releaseAbortController(abortController);
13821397
}
13831398
};
13841399
}
@@ -1394,23 +1409,21 @@ class Tangerine extends dns.promises.Resolver {
13941409
// <https://gist.github.com/andrewcourtice/ef1b8f14935b409cfe94901558ba5594#file-task-ts-L37>
13951410
// <https://github.com/nodejs/undici/blob/0badd390ad5aa531a66aacee54da664468aa1577/lib/api/api-fetch/request.js#L280-L295>
13961411
// <https://github.com/nodejs/node/issues/40849>
1412+
let mustReleaseAbortController = false;
13971413
if (!abortController) {
1398-
abortController = new AbortController();
1399-
this.abortControllers.add(abortController);
1400-
abortController.signal.addEventListener(
1401-
'abort',
1402-
() => {
1403-
this.abortControllers.delete(abortController);
1404-
},
1405-
{ once: true }
1406-
);
1414+
abortController = this.#createAbortController();
1415+
mustReleaseAbortController = true;
14071416

1408-
// <https://github.com/nodejs/undici/pull/1910/commits/7615308a92d3c8c90081fb99c55ab8bd59212396>
1409-
setMaxListeners(
1410-
getEventListeners(abortController.signal, 'abort').length +
1411-
this.constructor.ANY_TYPES.length,
1412-
abortController.signal
1413-
);
1417+
try {
1418+
// <https://github.com/nodejs/undici/pull/1910/commits/7615308a92d3c8c90081fb99c55ab8bd59212396>
1419+
setMaxListeners(
1420+
getEventListeners(abortController.signal, 'abort').length +
1421+
this.constructor.ANY_TYPES.length,
1422+
abortController.signal
1423+
);
1424+
} finally {
1425+
this.#releaseAbortController(abortController);
1426+
}
14141427
}
14151428

14161429
try {
@@ -1428,6 +1441,10 @@ class Tangerine extends dns.promises.Resolver {
14281441
err.syscall = 'queryAny';
14291442
err.message = `queryAny ${err.code} ${name}`;
14301443
throw err;
1444+
} finally {
1445+
if (mustReleaseAbortController) {
1446+
this.#releaseAbortController(abortController);
1447+
}
14311448
}
14321449
}
14331450

@@ -1674,20 +1691,20 @@ class Tangerine extends dns.promises.Resolver {
16741691
if (this.options.cache && result) {
16751692
debug(`cached result found for "${key}"`);
16761693
} else {
1694+
let mustReleaseAbortController = false;
16771695
if (!abortController) {
1678-
abortController = new AbortController();
1679-
this.abortControllers.add(abortController);
1680-
abortController.signal.addEventListener(
1681-
'abort',
1682-
() => {
1683-
this.abortControllers.delete(abortController);
1684-
},
1685-
{ once: true }
1686-
);
1696+
abortController = this.#createAbortController();
1697+
mustReleaseAbortController = true;
16871698
}
16881699

1689-
// setImmediate(() => this.cancel());
1690-
result = await this.#query(name, rrtype, ecsSubnet, abortController);
1700+
try {
1701+
// setImmediate(() => this.cancel());
1702+
result = await this.#query(name, rrtype, ecsSubnet, abortController);
1703+
} finally {
1704+
if (mustReleaseAbortController) {
1705+
this.#releaseAbortController(abortController);
1706+
}
1707+
}
16911708
}
16921709

16931710
// <https://github.com/m13253/dns-over-https/blob/2e36b4ebcdb8a1a102ea86370d7f8b1f1e72380a/json-dns/response.go#L50-L74>

0 commit comments

Comments
 (0)