Skip to content

Explicit Resource Management for pg pool client#3661

Open
hyperair wants to merge 16 commits intobrianc:masterfrom
hyperair:disposable-pool-client
Open

Explicit Resource Management for pg pool client#3661
hyperair wants to merge 16 commits intobrianc:masterfrom
hyperair:disposable-pool-client

Conversation

@hyperair
Copy link
Copy Markdown

@hyperair hyperair commented Apr 27, 2026

Add support for Explicit Resource Management for pg.Pool clients.

This is implemented using [Symbol.dispose] instead of [Symbol.asyncDispose] because client.release() is a synchronous function, not async.

Additionally, add a second property client.destroyOnDispose so that Symbol.dispose supports calling client.release(true). This is useful for a usecase that changes connection parameters in a way that is troublesome to reset upon release to the pool, e.g.

const pool = new Pool({
    database: 'foo',
    user: 'foo',
    password: 'foo',
    port: 5432,

    statement_timeout: 10_000,
});

async function getSlowClient() {
    const client = await pool.connect();
    await client.query("SET statement_timeout = 0");
    client.destroyOnDispose = true;
}

async function performReallySlowQuery() {
    using client = await getSlowClient();

    client.query(`select pg_sleep_for('30 minutes')`);
}

Fixes: #3515

Note: This PR has been rebased on top of #3662, so there are some irrelevant commits until that PR is merged.

@hyperair hyperair force-pushed the disposable-pool-client branch 3 times, most recently from bac8c73 to 7d69534 Compare April 27, 2026 05:05
@hyperair
Copy link
Copy Markdown
Author

Hmm, looks like eslint needs to be upgraded before it'll parse the using test

@hyperair hyperair marked this pull request as draft April 27, 2026 13:10
- rootDir defaults have changed, so we need to specify it manually now
- baseUrl is no longer supported
- types no longer loads everything in @types by default, so we have to specify
  that we want node types
- Pin @types/node to 16.* because we support node16 and above
Fixes the following error:

    % yarn build
    yarn run v1.22.19
    $ tsc --build
    packages/pg-cloudflare/src/index.ts:156:29 - error TS2769: No overload matches this call.
      The last overload gave the following error.
        Argument of type 'ArrayBuffer | Uint8Array<ArrayBufferLike>' is not assignable to parameter of type 'WithImplicitCoercion<string> | { [Symbol.toPrimitive](hint: "string"): string; }'.
          Type 'ArrayBuffer' is not assignable to type 'WithImplicitCoercion<string> | { [Symbol.toPrimitive](hint: "string"): string; }'.

    156     const hex = Buffer.from(data).toString('hex')
                                    ~~~~

      node_modules/@types/node/buffer.buffer.d.ts:83:13
         83             from(
                        ~~~~~
         84                 str:
            ~~~~~~~~~~~~~~~~~~~~
        ...
         89                 encoding?: BufferEncoding,
            ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
         90             ): Buffer<ArrayBuffer>;
            ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
        The last overload is declared here.

    Found 1 error.

See microsoft/TypeScript#63447 for more info
Fixes the following typescript error:

    node_modules/typescript/lib/lib.esnext.intl.d.ts:26:135 - error TS2552: Cannot find name 'DateTimeRangeFormatPart'. Did you mean 'DateTimeFormatPart'?

    26         formatRangeToParts(startDate: FormattableTemporalObject | Date | number, endDate: FormattableTemporalObject | Date | number): DateTimeRangeFormatPart[];
`BufferReader.encoding` to `BufferEncoding` from `string` to match the new
signature of `Buffer.toString`.
@hyperair hyperair force-pushed the disposable-pool-client branch from 0cb5f67 to 24caf46 Compare April 28, 2026 10:26
@hyperair hyperair marked this pull request as ready for review April 28, 2026 10:34
@hyperair hyperair requested a review from hjr3 as a code owner April 28, 2026 10:34
Copy link
Copy Markdown
Collaborator

@charmander charmander left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure about destroyOnDispose, i.e. whether its job is common enough to justify it and whether it’s the best API for the job. Would suggest initializing to false if kept. Note .release(true) continues to work for error cases given that double release is a no-op; I recognize that it doesn’t satisfy the use case you mentioned. Maybe release the feature without that API initially, since it can be added easily?

Edit: Sorry, I don’t know where I got that from – we haven’t even done PooledClient yet.

Comment thread packages/pg-pool/test/disposable-clients.js Outdated
Comment thread packages/pg-pool/index.js

client.release = this._releaseOnce(client, idleListener)

if (Symbol.dispose) {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this would normally go on the prototype, but pg-pool’s approach is already a bit messy (it’s lacking a PooledClient object that’s specific to one acquisition operation) so it’s also not the end of the world if the patch goes in as-is.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, and that's why I implemented it like this.

@types/pg has a PooledClient type with the release signature, and I intend to make a PR there to add the [Symbol.dispose] addition later.

Comment thread docs/pages/apis/pool.mdx Outdated
@hyperair
Copy link
Copy Markdown
Author

Not sure about destroyOnDispose, i.e. whether its job is common enough to justify it and whether it’s the best API for the job. Would suggest initializing to false if kept. Note .release(true) continues to work for error cases given that double release is a no-op; I recognize that it doesn’t satisfy the use case you mentioned. Maybe release the feature without that API initially, since it can be added easily?

Yeah I wasn't sure about the API either but I couldn't think of a better way to do it. Maybe a client.setError() to set an error state that client.release() acknowledges as well?

That said, unfortunately I don't think relying on double-release is workable either, because the second release, which will be called from within Symbol.dispose will throw. Even if [Symbol.dispose]() wraps that in a try-catch to swallow the error, needing to put the client.release(true) in a finally clause obviates any purpose of using the using syntax.

hyperair and others added 5 commits April 30, 2026 12:21
When `Symbol.dispose` is defined, define a disposer function that simply calls
`this.release()`. This lets the `PoolClient` with the `using` syntax work with any
downstream-overridden `release` functions.

This makes `PoolClient` support Explicit Resource Management when the runtime
supports it.

Fixes: brianc#3515
Add a `destroyOnDispose` property on the client to signal that
`[Symbol.dispose]()` should call `client.release(true)` instead of
`client.release()`.
Co-authored-by: Charmander <~@charmander.me>
`connect` has two `n`s, and we don't like semicolons

Co-authored-by: Charmander <~@charmander.me>
@hyperair hyperair force-pushed the disposable-pool-client branch from 36ef0cf to 8c8d974 Compare April 30, 2026 04:21
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.

Symbol.asyncDispose support

2 participants