Skip to content

Commit 27a495c

Browse files
titanismtitanism
authored
Merge pull request #13 from analog-nico/main
fix: memory leak introduced in [email protected]
2 parents 42da3ee + 5f4fead commit 27a495c

File tree

1 file changed

+143
-127
lines changed

1 file changed

+143
-127
lines changed

‎index.js‎

Lines changed: 143 additions & 127 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,120 +1282,116 @@ 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-
);
1288-
// wrap with try/catch because ENODATA shouldn't cause errors
1285+
const abortController = this.#createAbortController();
12891286
try {
1290-
switch (type) {
1291-
case 'A': {
1292-
const result = await this.resolve4(
1293-
name,
1294-
{ ...options, ttl: true },
1295-
abortController
1296-
);
1297-
return result.map((r) => ({ type, ...r }));
1298-
}
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+
// wrap with try/catch because ENODATA shouldn't cause errors
1299+
try {
1300+
switch (type) {
1301+
case 'A': {
1302+
const result = await this.resolve4(
1303+
name,
1304+
{ ...options, ttl: true },
1305+
abortController
1306+
);
1307+
return result.map((r) => ({ type, ...r }));
1308+
}
12991309

1300-
case 'AAAA': {
1301-
const result = await this.resolve6(
1302-
name,
1303-
{ ...options, ttl: true },
1304-
abortController
1305-
);
1306-
return result.map((r) => ({ type, ...r }));
1307-
}
1310+
case 'AAAA': {
1311+
const result = await this.resolve6(
1312+
name,
1313+
{ ...options, ttl: true },
1314+
abortController
1315+
);
1316+
return result.map((r) => ({ type, ...r }));
1317+
}
13081318

1309-
case 'CNAME': {
1310-
const result = await this.resolveCname(
1311-
name,
1312-
options,
1313-
abortController
1314-
);
1315-
return result.map((value) => ({ type, value }));
1316-
}
1319+
case 'CNAME': {
1320+
const result = await this.resolveCname(
1321+
name,
1322+
options,
1323+
abortController
1324+
);
1325+
return result.map((value) => ({ type, value }));
1326+
}
13171327

1318-
case 'MX': {
1319-
const result = await this.resolveMx(name, options, abortController);
1320-
return result.map((r) => ({ type, ...r }));
1321-
}
1328+
case 'MX': {
1329+
const result = await this.resolveMx(name, options, abortController);
1330+
return result.map((r) => ({ type, ...r }));
1331+
}
13221332

1323-
case 'NAPTR': {
1324-
const result = await this.resolveNaptr(
1325-
name,
1326-
options,
1327-
abortController
1328-
);
1329-
return result.map((value) => ({ type, value }));
1330-
}
1333+
case 'NAPTR': {
1334+
const result = await this.resolveNaptr(
1335+
name,
1336+
options,
1337+
abortController
1338+
);
1339+
return result.map((value) => ({ type, value }));
1340+
}
13311341

1332-
case 'NS': {
1333-
const result = await this.resolveNs(name, options, abortController);
1334-
return result.map((value) => ({ type, value }));
1335-
}
1342+
case 'NS': {
1343+
const result = await this.resolveNs(name, options, abortController);
1344+
return result.map((value) => ({ type, value }));
1345+
}
13361346

1337-
case 'PTR': {
1338-
const result = await this.resolvePtr(
1339-
name,
1340-
options,
1341-
abortController
1342-
);
1343-
return result.map((value) => ({ type, value }));
1344-
}
1347+
case 'PTR': {
1348+
const result = await this.resolvePtr(
1349+
name,
1350+
options,
1351+
abortController
1352+
);
1353+
return result.map((value) => ({ type, value }));
1354+
}
13451355

1346-
case 'SOA': {
1347-
const result = await this.resolveSoa(
1348-
name,
1349-
options,
1350-
abortController
1351-
);
1352-
return { type, ...result };
1353-
}
1356+
case 'SOA': {
1357+
const result = await this.resolveSoa(
1358+
name,
1359+
options,
1360+
abortController
1361+
);
1362+
return { type, ...result };
1363+
}
13541364

1355-
case 'SRV': {
1356-
const result = await this.resolveSrv(
1357-
name,
1358-
options,
1359-
abortController
1360-
);
1361-
return result.map((value) => ({ type, value }));
1362-
}
1365+
case 'SRV': {
1366+
const result = await this.resolveSrv(
1367+
name,
1368+
options,
1369+
abortController
1370+
);
1371+
return result.map((value) => ({ type, value }));
1372+
}
13631373

1364-
case 'TXT': {
1365-
const result = await this.resolveTxt(
1366-
name,
1367-
options,
1368-
abortController
1369-
);
1370-
return result.map((entries) => ({ type, entries }));
1371-
}
1374+
case 'TXT': {
1375+
const result = await this.resolveTxt(
1376+
name,
1377+
options,
1378+
abortController
1379+
);
1380+
return result.map((entries) => ({ type, entries }));
1381+
}
13721382

1373-
default: {
1374-
break;
1383+
default: {
1384+
break;
1385+
}
13751386
}
1376-
}
1377-
} catch (err) {
1378-
debug(err);
1387+
} catch (err) {
1388+
debug(err);
13791389

1380-
if (err.code === dns.NODATA) return;
1381-
throw err;
1390+
if (err.code === dns.NODATA) return;
1391+
throw err;
1392+
}
1393+
} finally {
1394+
this.#releaseAbortController(abortController);
13821395
}
13831396
};
13841397
}
@@ -1394,23 +1407,22 @@ class Tangerine extends dns.promises.Resolver {
13941407
// <https://gist.github.com/andrewcourtice/ef1b8f14935b409cfe94901558ba5594#file-task-ts-L37>
13951408
// <https://github.com/nodejs/undici/blob/0badd390ad5aa531a66aacee54da664468aa1577/lib/api/api-fetch/request.js#L280-L295>
13961409
// <https://github.com/nodejs/node/issues/40849>
1410+
let mustReleaseAbortController = false;
13971411
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-
);
1412+
abortController = this.#createAbortController();
1413+
mustReleaseAbortController = true;
14071414

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-
);
1415+
try {
1416+
// <https://github.com/nodejs/undici/pull/1910/commits/7615308a92d3c8c90081fb99c55ab8bd59212396>
1417+
setMaxListeners(
1418+
getEventListeners(abortController.signal, 'abort').length +
1419+
this.constructor.ANY_TYPES.length,
1420+
abortController.signal
1421+
);
1422+
} catch (err) {
1423+
this.#releaseAbortController(abortController);
1424+
throw err;
1425+
}
14141426
}
14151427

14161428
try {
@@ -1428,6 +1440,10 @@ class Tangerine extends dns.promises.Resolver {
14281440
err.syscall = 'queryAny';
14291441
err.message = `queryAny ${err.code} ${name}`;
14301442
throw err;
1443+
} finally {
1444+
if (mustReleaseAbortController) {
1445+
this.#releaseAbortController(abortController);
1446+
}
14311447
}
14321448
}
14331449

@@ -1674,20 +1690,20 @@ class Tangerine extends dns.promises.Resolver {
16741690
if (this.options.cache && result) {
16751691
debug(`cached result found for "${key}"`);
16761692
} else {
1693+
let mustReleaseAbortController = false;
16771694
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-
);
1695+
abortController = this.#createAbortController();
1696+
mustReleaseAbortController = true;
16871697
}
16881698

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

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

0 commit comments

Comments
 (0)