Skip to content

Commit

Permalink
fix(transport): remove listeners of AbortSignal to prevent memory leaks
Browse files Browse the repository at this point in the history
  • Loading branch information
szymonlesisz authored and mroz22 committed Jul 7, 2024
1 parent 62c2c5c commit b7b51d8
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 15 deletions.
6 changes: 4 additions & 2 deletions packages/transport-bridge/src/http.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,9 +96,11 @@ export class TrezordNode {

private createAbortSignal(res: any) {
const abortController = new AbortController();
res.addListener('close', () => {
const listener = () => {
abortController.abort();
});
res.removeListener('close', listener);
};
res.addListener('close', listener);

return abortController.signal;
}
Expand Down
30 changes: 17 additions & 13 deletions packages/transport/src/api/udp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,15 +42,18 @@ export class UdpApi extends AbstractApi {
const [hostname, port] = path.split(':');

return new Promise<AbstractApiAwaitedResult<'write'>>(resolve => {
signal?.addEventListener('abort', () => {
const listener = () => {
resolve(
this.error({
error: ERRORS.ABORTED_BY_SIGNAL,
}),
);
});
};
signal?.addEventListener('abort', listener);

this.interface.send(buffer, Number.parseInt(port, 10), hostname, err => {
signal?.removeEventListener('abort', listener);

if (signal?.aborted) {
return;
}
Expand All @@ -75,13 +78,13 @@ export class UdpApi extends AbstractApi {
this.communicating = true;

return new Promise<AbstractApiAwaitedResult<'read'>>(resolve => {
/* eslint-disable @typescript-eslint/no-use-before-define */
const onClear = () => {
// eslint-disable-next-line @typescript-eslint/no-use-before-define
this.interface.removeListener('error', onError);
// eslint-disable-next-line @typescript-eslint/no-use-before-define
this.interface.removeListener('message', onMessage);
signal?.removeEventListener('abort', onAbort);
};

/* eslint-enable @typescript-eslint/no-use-before-define */
const onError = (err: Error) => {
this.logger?.error(err.message);

Expand Down Expand Up @@ -126,21 +129,22 @@ export class UdpApi extends AbstractApi {
}

const pinged = new Promise<boolean>(resolve => {
const onError = () => {
resolve(false);
/* eslint-disable @typescript-eslint/no-use-before-define */
const onClear = () => {
this.interface.removeListener('error', onError);
// eslint-disable-next-line @typescript-eslint/no-use-before-define
this.interface.removeListener('message', onMessage);
// eslint-disable-next-line @typescript-eslint/no-use-before-define
clearTimeout(timeout);
signal?.removeEventListener('abort', onError);
};
/* eslint-enable @typescript-eslint/no-use-before-define */
const onError = () => {
resolve(false);
onClear();
};
const onMessage = (message: Buffer, _info: UDP.RemoteInfo) => {
if (message.toString() === 'PONGPONG') {
resolve(true);
this.interface.removeListener('error', onError);
this.interface.removeListener('message', onMessage);
// eslint-disable-next-line @typescript-eslint/no-use-before-define
clearTimeout(timeout);
onClear();
}
};

Expand Down

0 comments on commit b7b51d8

Please sign in to comment.