Skip to content

Commit 55e3f5d

Browse files
authored
fix: connector close leaks for in-flight queries (#566)
1 parent a06c08d commit 55e3f5d

2 files changed

Lines changed: 27 additions & 16 deletions

File tree

src/cloud-sql-instance.ts

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -364,12 +364,14 @@ export class CloudSQLInstance {
364364
this.checkDomainID = null;
365365
}
366366
for (const socket of this.sockets) {
367-
socket.destroy(
368-
new CloudSQLConnectorError({
369-
code: 'ERRCLOSED',
370-
message: 'The connector was closed.',
371-
})
372-
);
367+
if (typeof socket.destroy === 'function') {
368+
socket.destroy(
369+
new CloudSQLConnectorError({
370+
code: 'ERRCLOSED',
371+
message: 'The connector was closed.',
372+
})
373+
);
374+
}
373375
}
374376
}
375377

@@ -391,15 +393,15 @@ export class CloudSQLInstance {
391393
}
392394
}
393395
addSocket(socket: DestroyableSocket) {
394-
if (!this.instanceInfo.domainName) {
395-
// This was not connected by domain name. Ignore all sockets.
396-
return;
397-
}
398-
399-
// Add the socket to the list
396+
// Track all active sockets created by this instance so they can
397+
// be forcefully cleaned up during a domain change or when
398+
// the connector is explicitly closed.
400399
this.sockets.add(socket);
401-
// When the socket is closed, remove it.
402-
socket.once('closed', () => {
400+
401+
// When the socket is closed by the driver or peer, remove it
402+
// from our tracking set to prevent reference memory leaks.
403+
// Note: Node.js TLSSocket/Socket emits 'close', not 'closed'.
404+
socket.once('close', () => {
403405
this.sockets.delete(socket);
404406
});
405407
}

src/connector.ts

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -358,8 +358,17 @@ export class Connector {
358358
//
359359
// Also clear up any local proxy servers and socket connections.
360360
close(): void {
361-
for (const instance of this.instances.values()) {
362-
instance.promise.then(inst => inst.close());
361+
for (const entry of this.instances.values()) {
362+
if (entry.isResolved() && entry.instance) {
363+
// If the instance is already resolved, close it synchronously.
364+
// This prevents a race condition with immediate connection pool close
365+
// (e.g., pool.end()) where asynchronous microtasks would execute too late.
366+
entry.instance.close();
367+
} else {
368+
// Otherwise, close the instance asynchronously once its initial
369+
// refresh has finished.
370+
entry.promise.then(inst => inst.close()).catch(() => {});
371+
}
363372
}
364373
for (const server of this.localProxies) {
365374
server.close();

0 commit comments

Comments
 (0)