Skip to content

Commit 2dfa6f3

Browse files
committed
fix: prevent memory leaks in socket and event handling
- Convert anonymous streamer error handler to named function for proper cleanup - Add explicit cleanup of connection error handler in setSocketHandlers() - Clean up socketPlain error handler after successful STARTTLS upgrade - Add explicit nullification of socket/stream references in close() - Add comprehensive memory leak tests for connection cycles Fixes potential memory leaks when creating/destroying many IMAP connections.
1 parent ae2ebcf commit 2dfa6f3

File tree

2 files changed

+396
-8
lines changed

2 files changed

+396
-8
lines changed

lib/imap-flow.js

Lines changed: 43 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -372,7 +372,8 @@ class ImapFlow extends EventEmitter {
372372

373373
this.disableBinary = !!this.options.disableBinary;
374374

375-
this.streamer.on('error', err => {
375+
// Named error handler for proper cleanup
376+
this._streamerErrorHandler = err => {
376377
if (['Z_BUF_ERROR', 'ECONNRESET', 'EPIPE', 'ETIMEDOUT', 'EHOSTUNREACH'].includes(err.code)) {
377378
// just close the connection, usually nothing but noise
378379
this.closeAfter();
@@ -381,7 +382,8 @@ class ImapFlow extends EventEmitter {
381382

382383
this.log.error({ err, cid: this.id });
383384
this.emitError(err);
384-
});
385+
};
386+
this.streamer.on('error', this._streamerErrorHandler);
385387

386388
// Has the `connect` method already been called
387389
this._connectCalled = false;
@@ -772,6 +774,12 @@ class ImapFlow extends EventEmitter {
772774
// Clear any existing handlers first to prevent duplicates
773775
this.clearSocketHandlers();
774776

777+
// Remove temporary connection error handler if present
778+
if (this._connectErrorHandler && this.socket) {
779+
this.socket.removeListener('error', this._connectErrorHandler);
780+
this._connectErrorHandler = null;
781+
}
782+
775783
this._socketError =
776784
this._socketError ||
777785
(err => {
@@ -828,6 +836,12 @@ class ImapFlow extends EventEmitter {
828836
return;
829837
}
830838

839+
// Remove temporary connection error handler if still present
840+
if (this._connectErrorHandler) {
841+
this.socket.removeListener('error', this._connectErrorHandler);
842+
this._connectErrorHandler = null;
843+
}
844+
831845
if (this._socketError) {
832846
this.socket.removeListener('error', this._socketError);
833847
this.socket.removeListener('tlsClientError', this._socketError);
@@ -1043,7 +1057,8 @@ class ImapFlow extends EventEmitter {
10431057
);
10441058
this.clearSocketHandlers();
10451059

1046-
socketPlain.once('error', err => {
1060+
// Store error handler for cleanup after successful upgrade
1061+
const socketPlainErrorHandler = err => {
10471062
clearTimeout(this.connectTimeout);
10481063
clearTimeout(this.upgradeTimeout);
10491064
if (!this.upgrading) {
@@ -1054,7 +1069,8 @@ class ImapFlow extends EventEmitter {
10541069
this.upgrading = false;
10551070
err.tlsFailed = true;
10561071
reject(err);
1057-
});
1072+
};
1073+
socketPlain.once('error', socketPlainErrorHandler);
10581074

10591075
this.upgradeTimeout = setTimeout(() => {
10601076
if (!this.upgrading) {
@@ -1093,6 +1109,9 @@ class ImapFlow extends EventEmitter {
10931109
});
10941110
}
10951111

1112+
// Clean up the plain socket error handler after successful upgrade
1113+
socketPlain.removeListener('error', socketPlainErrorHandler);
1114+
10961115
return resolve(true);
10971116
} catch (ex) {
10981117
this.emitError(ex);
@@ -1607,13 +1626,15 @@ class ImapFlow extends EventEmitter {
16071626

16081627
this.writeSocket = this.socket;
16091628

1610-
this.socket.on('error', err => {
1629+
// Store connection error handler for cleanup
1630+
this._connectErrorHandler = err => {
16111631
clearTimeout(this.connectTimeout);
16121632
clearTimeout(this.greetingTimeout);
16131633
this.closeAfter();
16141634
this.log.error({ err, cid: this.id });
16151635
reject(err);
1616-
});
1636+
};
1637+
this.socket.on('error', this._connectErrorHandler);
16171638
});
16181639
}
16191640

@@ -1761,11 +1782,13 @@ class ImapFlow extends EventEmitter {
17611782
// cleanup streamer
17621783
if (this.streamer) {
17631784
try {
1764-
// remove our listeners
1785+
// remove our listeners explicitly by reference
17651786
if (this.socketReadable) {
17661787
this.streamer.removeListener('readable', this.socketReadable);
17671788
}
1768-
this.streamer.removeAllListeners('error');
1789+
if (this._streamerErrorHandler) {
1790+
this.streamer.removeListener('error', this._streamerErrorHandler);
1791+
}
17691792
if (!this.streamer.destroyed) {
17701793
this.streamer.destroy();
17711794
}
@@ -1811,6 +1834,18 @@ class ImapFlow extends EventEmitter {
18111834
}
18121835
}
18131836

1837+
// Explicit nullification to help garbage collection
1838+
this.socket = null;
1839+
this.writeSocket = null;
1840+
this._inflate = null;
1841+
this._deflate = null;
1842+
this._streamerErrorHandler = null;
1843+
this._connectErrorHandler = null;
1844+
this._socketError = null;
1845+
this._socketClose = null;
1846+
this._socketEnd = null;
1847+
this._socketTimeout = null;
1848+
18141849
this.log.trace({ msg: 'Connection closed', cid: this.id });
18151850
this.emit('close');
18161851
} catch (ex) {

0 commit comments

Comments
 (0)