fix: double client.end() hang (#2717)

* fix: double client.end() hang

fixes https://github.com/brianc/node-postgres/issues/2716

`client.end()` will resolve early if the connection is already dead,
rather than waiting for an "end" event that will never arrive.

* fix: client.end() resolves when socket is fully closed
This commit is contained in:
Cody Greene 2023-03-06 10:10:07 -08:00 committed by GitHub
parent adbe86d4a0
commit 5703791640
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 41 additions and 4 deletions

View File

@ -37,6 +37,7 @@ class Client extends EventEmitter {
this._Promise = c.Promise || global.Promise
this._types = new TypeOverrides(c.types)
this._ending = false
this._ended = false
this._connecting = false
this._connected = false
this._connectionError = false
@ -132,6 +133,7 @@ class Client extends EventEmitter {
clearTimeout(this.connectionTimeoutHandle)
this._errorAllQueries(error)
this._ended = true
if (!this._ending) {
// if the connection is ended without us calling .end()
@ -603,7 +605,7 @@ class Client extends EventEmitter {
this._ending = true
// if we have never connected, then end is a noop, callback immediately
if (!this.connection._connecting) {
if (!this.connection._connecting || this._ended) {
if (cb) {
cb()
} else {

View File

@ -108,9 +108,6 @@ class Connection extends EventEmitter {
}
attachListeners(stream) {
stream.on('end', () => {
this.emit('end')
})
parse(stream, (msg) => {
var eventName = msg.name === 'error' ? 'errorMessage' : msg.name
if (this._emitMessage) {

View File

@ -0,0 +1,38 @@
'use strict'
const helper = require('../test-helper')
const suite = new helper.Suite()
// https://github.com/brianc/node-postgres/issues/2716
suite.testAsync('client.end() should resolve if already ended', async () => {
const client = new helper.pg.Client()
await client.connect()
// this should resolve only when the underlying socket is fully closed, both
// the readable part ("end" event) & writable part ("close" event).
// https://nodejs.org/docs/latest-v16.x/api/net.html#event-end
// > Emitted when the other end of the socket signals the end of
// > transmission, thus ending the readable side of the socket.
// https://nodejs.org/docs/latest-v16.x/api/net.html#event-close_1
// > Emitted once the socket is fully closed.
// here: stream = socket
await client.end()
// connection.end()
// stream.end()
// ...
// stream emits "end"
// not listening to this event anymore so the promise doesn't resolve yet
// stream emits "close"; no more events will be emitted from the stream
// connection emits "end"
// promise resolved
// This should now resolve immediately, rather than wait for connection.on('end')
await client.end()
// this should resolve immediately, rather than waiting forever
await client.end()
})