drizzle-team / drizzle-team/drizzle-orm

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

Open
#5,839 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
TypeScript
Stars
35.8k
Forks
1.6k
Avg merge
2d 7h
Merged PRs (30d)
4

Description

## Description

The `iterator()` method in both `mysql2` and `singlestore` session implementations (`drizzle-orm/src/mysql2/session.ts` and `drizzle-orm/src/singlestore/session.ts`) leaves dangling `once()` event listeners on the underlying stream when the loop exits. This leads to `MaxListenersExceededWarning` in long-running applications that call the iterator many times, and can also cause spurious unhandled-error events after the iterator is done.

## Root Cause

The iterator sets up two persistent `once()` promises before the loop:

```ts
const onEnd = once(stream, 'end');
const onError = once(stream, 'error');
```

These two calls each attach a listener to the stream. The listeners are only removed when the respective event fires. However, the loop can exit through the `data` path (normal row consumption) without the `end` or `error` events being consumed, so those listeners remain attached indefinitely.

Similarly, inside each loop iteration a new per-iteration data listener is registered:

```ts
new Promise((resolve) => stream.once('data', resolve))
```

If `onEnd` or `onError` wins the `Promise.race` for that iteration, the `once('data', ...)` listener is never removed — `once()` only self-removes when the event fires, not when the race is lost.

The `finally` block only removes the permanent `dataListener` added via `stream.on('data', dataListener)`. It does not cancel the `onEnd`/`onError` promises or their underlying listeners, and it does not remove losing per-iteration `once('data', ...)` listeners.

Affected lines (identical pattern in both drivers):

```
drizzle-orm/src/mysql2/session.ts – iterator(), roughly lines 150-195
drizzle-orm/src/singlestore/session.ts – iterator(), same pattern
```

## Steps to Reproduce

```ts
import { drizzle } from 'drizzle-orm/mysql2';
import mysql from 'mysql2/promise';

const pool = mysql.createPool({ /* ... */ });
const db = drizzle(pool);

// Run the iterator many times (e.g. in a loop or under load)
for (let i = 0; i < 20; i++) {
const stmt = db.select().from(myTable).prepare('q');
for await (const _row of stmt.iterator()) {
// consume all rows normally
}
}
// Node.js will emit MaxListenersExceededWarning after ~11 iterations
// because 'end' and 'error' once-listeners accumulate on the stream.
```

## Expected Behaviour

All event listeners added during `iterator()` are removed when the generator returns or throws, regardless of which `Promise.race` branch caused the loop to exit.

## Actual Behaviour

- `once(stream, 'end')` leaves a dangling `end` listener whenever the loop exits via a `data` event (i.e. normal row consumption followed by the `undefined`/empty-array break).
- `once(stream, 'error')` leaves a dangling `error` listener on every successful iteration.
- Per-iteration `stream.once('data', resolve)` leaves a dangling `data` listener whenever `onEnd` or `onError` wins the race.

In applications that reuse the stream object or call the iterator frequently, Node.js emits `MaxListenersExceededWarning` and memory usage grows proportionally to the number of iterator calls.

## Suggested Fix

Use `node:events` `EventEmitter.removeListener` (or the `AbortController`/`signal` overload of `once()`) to clean up all listeners in the `finally` block. A minimal approach:

```ts
const ac = new AbortController();
const { signal } = ac;

try {
const onEnd = once(stream, 'end', { signal });
const onError = once(stream, 'error', { signal });

while (true) {
stream.resume();
const row = await Promise.race([
onEnd,
onError,
new Promise((resolve) => stream.once('data', resolve)),
]);
// ...
}
} finally {
ac.abort(); // cancels onEnd + onError listeners
stream.off('data', dataListener);
if (isPool(client)) conn.end();
}
```

Alternatively, track and manually remove all registered listeners in the `finally` block.

Note: the per-iteration `stream.once('data', resolve)` race-loser listener also needs handling. One idiomatic solution is to keep a reference to the resolve callback and call `stream.off('data', resolveRef)` at the top of each loop iteration before registering a new one.

## Environment

- drizzle-orm: current `main` (both `mysql2` and `singlestore` drivers share the identical pattern)
- Node.js: any version that supports `EventEmitter.once()` (v12+)
- mysql2: any version that exposes a streaming query API

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.