Skip to content

[Mysql][SingleStore]: fix dangling event listeners in iterator()#5856

Open
sijie-Z wants to merge 1 commit into
drizzle-team:mainfrom
sijie-Z:fix/cleanup-iterator-listeners
Open

[Mysql][SingleStore]: fix dangling event listeners in iterator()#5856
sijie-Z wants to merge 1 commit into
drizzle-team:mainfrom
sijie-Z:fix/cleanup-iterator-listeners

Conversation

@sijie-Z

@sijie-Z sijie-Z commented Jun 7, 2026

Copy link
Copy Markdown

Summary

Fixes #5839

Problem

The iterator() method in both mysql2 and singlestore session implementations leaves dangling once() event listeners on the underlying stream. When the generator exits (normally or via error), once(stream, 'end') and once(stream, 'error') listeners remain attached. Per-iteration stream.once('data', resolve) listeners also leak when another branch of Promise.race wins.

In long-running applications, this causes MaxListenersExceededWarning and memory growth proportional to iterator call count.

Solution

  • Use AbortController to register once() listeners with a signal, then call ac.abort() in the finally block to remove them
  • Track per-iteration data listener references and clean them up at the start of each loop iteration before registering a new one

Applied identically to both drizzle-orm/src/mysql2/session.ts and drizzle-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.

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
@Nexory

Nexory commented Jun 7, 2026

Copy link
Copy Markdown

The AbortController + signal part closes the onEnd and onError leaks cleanly — that's the bulk of the listener accumulation.

One residual gap on the final iteration: when the loop exits via the break path (onEnd wins the last Promise.race, row resolves to undefined/empty) or via throw (onError wins), execution jumps straight to finally without re-entering the top of the loop. At that point dataResolve still holds the stream.once('data', resolve) from the last iteration. That listener was registered with no signal, so ac.abort() doesn't touch it, and the finally block only removes the permanent dataListener. So exactly one 'data' listener leaks per iterator() call — MaxListenersExceededWarning will still accumulate, just at roughly one-third the original rate.

Mirroring the per-iteration cleanup pattern into finally should close it:

} finally {
    ac.abort();
    stream.off('data', dataListener);
    if (dataResolve) {
        stream.off('data', dataResolve);
    }
    if (isPool(client)) {
        conn.end();
    }
}

Same change in both mysql2/session.ts (~line 202) and singlestore/session.ts (~line 201).

Happy to send a follow-up PR with this plus a unit-level reproduction that pins listenerCount on a mocked EventEmitter stream.

Nexory added a commit to Nexory/drizzle-orm that referenced this pull request Jun 7, 2026
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).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

mysql2 / singlestore iterator leaves dangling once() event listeners on stream after loop exits

2 participants