[Mysql][SingleStore]: fix dangling event listeners in iterator()#5856
[Mysql][SingleStore]: fix dangling event listeners in iterator()#5856sijie-Z wants to merge 1 commit into
Conversation
Use AbortController to cancel once() listeners in the finally block, and track per-iteration data listener references for cleanup between iterations. Prevents MaxListenersExceededWarning in long-running apps. Fixes drizzle-team#5839
|
The One residual gap on the final iteration: when the loop exits via the Mirroring the per-iteration cleanup pattern into } finally {
ac.abort();
stream.off('data', dataListener);
if (dataResolve) {
stream.off('data', dataResolve);
}
if (isPool(client)) {
conn.end();
}
}Same change in both Happy to send a follow-up PR with this plus a unit-level reproduction that pins |
Closes drizzle-team#5839. iterator() attaches three categories of listener to the underlying query stream (the permanent data listener, once(stream, 'end' | 'error'), and a per-iteration stream.once('data', resolve)) and the previous finally block removed only the permanent data listener. Repeated calls accumulate listeners on the connection-level stream until Node emits MaxListenersExceededWarning. This commit: - Wraps once(stream, 'end') and once(stream, 'error') with an AbortController signal so ac.abort() in the finally block removes them on every exit path (data wins, end wins, error wins, consumer break, consumer throw). - Tracks each iteration's stream.once('data', resolve) callback in a dataResolve variable, removes the previous one at the top of the next loop iteration, AND removes the final-iteration one in the finally block. The final-iteration removal is what closes the residual leak that PR drizzle-team#5856 left behind. The AbortController + signal pattern and the per-iteration dataResolve tracking were the suggested fix in issue drizzle-team#5839; this PR adds the missing final-iteration cleanup and tests. Tested via a new unit test file that builds a fake mysql2 client over an EventEmitter and asserts stream.listenerCount('data' | 'end' | 'error') returns to 0 after every exit path. A 20-iteration shared-stream test additionally pins that process never emits MaxListenersExceededWarning. Empirical delta on drizzle-orm/tests/iterator-listener-cleanup.test.ts: pre-fix current main: 6 failed | 0 passed (6) partial fix from PR drizzle-team#5856: 5 failed | 1 passed (6) this commit: 0 failed | 6 passed (6) Full vitest suite for drizzle-orm: 572 passed (572).
Summary
Fixes #5839
Problem
The
iterator()method in bothmysql2andsinglestoresession implementations leaves danglingonce()event listeners on the underlying stream. When the generator exits (normally or via error),once(stream, 'end')andonce(stream, 'error')listeners remain attached. Per-iterationstream.once('data', resolve)listeners also leak when another branch ofPromise.racewins.In long-running applications, this causes
MaxListenersExceededWarningand memory growth proportional to iterator call count.Solution
AbortControllerto registeronce()listeners with asignal, then callac.abort()in thefinallyblock to remove themdatalistener references and clean them up at the start of each loop iteration before registering a new oneApplied identically to both
drizzle-orm/src/mysql2/session.tsanddrizzle-orm/src/singlestore/session.ts.Testing
Existing iterator tests (
select iterator,select iterator w/ prepared statement) cover the happy path. The fix is a resource cleanup change — no behavior change for consumed iterators, only prevents listener accumulation on stream exit.