Skip to content

fix(pg, pg-pool): clamp timer delays to what setTimeout supports - #3817

Open
OsamaAnsar wants to merge 1 commit into
brianc:masterfrom
OsamaAnsar:fix/clamp-timer-delays
Open

OsamaAnsar wants to merge 1 commit into
brianc:masterfrom
OsamaAnsar:fix/clamp-timer-delays

Conversation

@OsamaAnsar

Copy link
Copy Markdown

setTimeout() treats a delay above 2^31-1 ms (about 24.8 days) as 1 ms, so a large maxLifetimeSeconds, idleTimeoutMillis, connectionTimeoutMillis or query_timeout made its timer fire almost immediately. This clamps the delay at each setTimeout call (pg-pool x4, Client x2, and the native client's query_timeout). Values that were already in range are unchanged.

Before/after, real pg + pg-pool from this checkout against PGlite, 6 sequential select 1 on a pool of 1:

option before after
maxLifetimeSeconds: 2592000 6 connections opened, 6 removed 1 opened, 0 removed
idleTimeoutMillis: 3e9 6 opened, 6 removed 1 opened, 0 removed
idleTimeoutMillis: Infinity 6 opened, 6 removed 1 opened, 0 removed
query_timeout: 2**31, pg_sleep(0.3) Query read timeout after 302 ms OK
connectionTimeoutMillis: 2**31 (client) timeout expired after 2 ms OK

Tests: packages/pg-pool/test/timer-overflow.js (mock client, no database) and four cases in packages/pg/test/unit/client/query-timeout-tests.js (record the delay passed to setTimeout). With only the source change reverted, all 5 pool tests fail and the new pg unit tests fail; with the fix they pass. ESLint/prettier clean on the touched packages.

Not run: the pg-pool and pg integration suites (need a live Postgres), and the native client change (no pg-native here). Tested on Node 24 only; the change is Math.min on a number, so I don't expect it to differ on Bun, Deno or Workers, but I haven't run it there.

Open PRs #3721/#3711 (pg-pool connectionTimeoutMillis) and #3815 (query_timeout in client.js) edit lines next to these, so expect a small conflict with whichever lands second.

setTimeout() treats a delay above 2^31-1 ms (about 24.8 days) as 1 ms, so a
large maxLifetimeSeconds, idleTimeoutMillis, connectionTimeoutMillis or
query_timeout made its timer fire almost immediately. Clamp the delay at each
call site.
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