From 27c545b04292611ba73c92d5ee34409c7ed7733a Mon Sep 17 00:00:00 2001
From: Cody Greene <cody-greene@users.noreply.github.com>
Date: Sun, 6 Mar 2022 15:57:29 -0800
Subject: [PATCH 1/2] 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.
---
 packages/pg/lib/client.js                     |  4 +++-
 .../test/integration/gh-issues/2716-tests.js  | 20 +++++++++++++++++++
 2 files changed, 23 insertions(+), 1 deletion(-)
 create mode 100644 packages/pg/test/integration/gh-issues/2716-tests.js

diff --git a/packages/pg/lib/client.js b/packages/pg/lib/client.js
index 589aa9f84..b55573ce8 100644
--- a/packages/pg/lib/client.js
+++ b/packages/pg/lib/client.js
@@ -38,6 +38,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
@@ -133,6 +134,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()
@@ -589,7 +591,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 {
diff --git a/packages/pg/test/integration/gh-issues/2716-tests.js b/packages/pg/test/integration/gh-issues/2716-tests.js
new file mode 100644
index 000000000..73936e76d
--- /dev/null
+++ b/packages/pg/test/integration/gh-issues/2716-tests.js
@@ -0,0 +1,20 @@
+'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()
+  await client.end()
+  // connection "end" event is emitted twice; once on stream "close" and once
+  // on stream "end" so we need to make sure our second client.end() is not
+  // resolved by these.
+  await sleep(1)
+  await client.end() // this should resolve early, rather than waiting forever
+})
+
+function sleep(ms) {
+  return new Promise((resolve) => setTimeout(resolve, ms))
+}

From 533e529346babe6a0eb7b26b9355463e0a890ae3 Mon Sep 17 00:00:00 2001
From: Cody Greene <cody-greene@users.noreply.github.com>
Date: Thu, 17 Mar 2022 16:37:17 -0700
Subject: [PATCH 2/2] fix: client.end() resolves when socket is fully closed

---
 packages/pg/lib/connection.js                 |  3 --
 .../test/integration/gh-issues/2716-tests.js  | 36 ++++++++++++++-----
 2 files changed, 27 insertions(+), 12 deletions(-)

diff --git a/packages/pg/lib/connection.js b/packages/pg/lib/connection.js
index ebb2f099d..e4dc1199d 100644
--- a/packages/pg/lib/connection.js
+++ b/packages/pg/lib/connection.js
@@ -103,9 +103,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) {
diff --git a/packages/pg/test/integration/gh-issues/2716-tests.js b/packages/pg/test/integration/gh-issues/2716-tests.js
index 73936e76d..62d0942ba 100644
--- a/packages/pg/test/integration/gh-issues/2716-tests.js
+++ b/packages/pg/test/integration/gh-issues/2716-tests.js
@@ -7,14 +7,32 @@ const suite = new helper.Suite()
 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()
-  // connection "end" event is emitted twice; once on stream "close" and once
-  // on stream "end" so we need to make sure our second client.end() is not
-  // resolved by these.
-  await sleep(1)
-  await client.end() // this should resolve early, rather than waiting forever
-})
 
-function sleep(ms) {
-  return new Promise((resolve) => setTimeout(resolve, ms))
-}
+  // this should resolve immediately, rather than waiting forever
+  await client.end()
+})