[bizob2828]

This was related to a bug logged in newrelic.

A pooled client's underlying socket is a long-lived native resource.
Node permanently associates whatever AsyncLocalStorage context was
active when the socket connected with that resource, and later events
on it -- including every subsequent query's response -- are delivered
within that same, now-stale context, not the context of whoever
actually checked the client out this time.

This has two consequences. An application using its own
AsyncLocalStorage around the pool observes later queries' callbacks
running in the wrong context (#2533). And any AsyncLocalStorage-based
instrumentation that stores data in its context (e.g. APM request
tracing) keeps whatever was stored the first time a connection opened
reachable in memory for the connection's entire life, since it's part
of what got captured.

Reset each client's socket handle's async-context association via
handle.asyncReset() whenever it's handed out for reuse, and once right
after it finishes connecting. This is the same API and pattern Node's
own http.Agent uses when handing a pooled, already-connected socket to
a new request (lib/_http_agent.js, asyncResetHandle).

The AsyncResource captured at connect()/query() time to replay the
caller's original context on handoff is dropped from the PendingItem
immediately after dispatch, rather than left there -- otherwise the
resource itself (and whatever context it captured) stays reachable for
however long something else in the connection's internal listener graph
happens to hold onto that PendingItem.

[bizob2828]

@brianc I don't think the CI failure is related to my change but please let me know if I unknowingly messed something up

[charmander]

Does this do anything that isn’t already covered by the other two tests? Don’t want to reach into Node internals unnecessarily, even for test code.

[bizob2828]

this is a test for Node < 24 and to cover this code. Basically in Node 24 AsyncLocalStorage now relies on AsyncContextFrame, see

[charmander]

Thanks for the link. I won’t get the chance to look into it properly anytime soon, but that really seems like the kind of code that should not be added.

[bizob2828]

I'm not sure I understand. That code is necessary to handle the issue in node < 24. looking at your engines stanza, you support 16+. I can try to provide another minimal repro to demonstrate it better. FWIW there are several other issues with retaining objects during this pool connection reuse in versions less than 24 but I had to limit this PR as the others touched the pg client and some other package.

[charmander]

Reaching into Node implementation details, especially in this way, is rarely the best or only way to solve a problem. In the case – which seems like it should be unlikely, again with the qualification that I haven’t looked into it – that it truly is the only way to solve the problem, it might be better left as a workaround to be implemented by users of pg on Node <24 who also use AsyncLocalStorage.

Updating symbol-keyed properties by string representation is really questionable.

[bizob2828]

That's fair. Please still dig into it though. I'll check/explore alternate options or as you said, just document the workaround. Anyone that uses any observability tool will be implicitly using als. But the agents could instrument this if it is indeed an actual issue. Most, if not all of these changes mirror what HttpAgent is doing with sockets to avoid this exact issue.