Skip to content

fix(pg-cursor): settle every read when reads overlap - #3813

Open
Kesavamoorthig06 wants to merge 1 commit into
brianc:masterfrom
Kesavamoorthig06:fix-pg-cursor-concurrent-reads
Open

Kesavamoorthig06 wants to merge 1 commit into
brianc:masterfrom
Kesavamoorthig06:fix-pg-cursor-concurrent-reads

Conversation

@Kesavamoorthig06

Copy link
Copy Markdown

Problem

When two read() calls on the same Cursor overlap, the first promise never settles.

Repro (against any running postgres):

const { Client } = require('pg')
const Cursor = require('pg-cursor')

const client = new Client({ connectionString: process.env.DATABASE_URL })
await client.connect()
const cursor = client.query(new Cursor('select 1 union all select 2'))

const a = cursor.read(1)
const b = cursor.read(1)
await b          // resolves: [ { "?column?": 1 } ]
await a          // hangs forever

Cause

The second read() submits while the connection is still busy with the first read's portal. handleRowDescription (and _ifNoData) unconditionally shift the submission queue and overwrite this._cb while this._state === 'busy', clobbering the first read's callback. On top of that, _sendRows returns after delivering rows without shifting the queue, so a read that was queued behind an in-flight read is never started when the connection goes idle.

Fix

  • In handleRowDescription / _ifNoData, only shift the queue and install _cb when the connection is not busy; a busy connection means the next read's portal was already sent by next and the shift happened there.
  • In _sendRows, after delivering rows, if the connection is idle and more reads are queued, start the next one via _shiftQueue.

Adds a regression test (concurrent reads settle in order) that fails without the fix (mocha 2s timeout) and passes with it.

Two overlapping read() calls corrupted the cursor state machine: the
row description handler shifted the queued read while the first read
was still busy, overwriting the active callback so the first read
never settled, and the batch delivery path never drained the queue.

Guard the queue shifts in handleRowDescription/_ifNoData against the
busy state and drain the queue from _sendRows once the active read
settles.
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.

1 participant