Fix double-sync crash on postgres 9.x

brianc · brianc · commit 0bc0d9a7130a · 2020-10-07T14:50:12.000-05:00
diff --git a/packages/pg/lib/query.js b/packages/pg/lib/query.js
@@ -31,6 +31,7 @@ class Query extends EventEmitter {
     this.isPreparedStatement = false
     this._canceledDueToError = false
     this._promise = null
+    this._hasSentSync = false
   }
 
   requiresPreparation() {
@@ -100,7 +101,8 @@ class Query extends EventEmitter {
     this._checkForMultirow()
     this._result.addCommandComplete(msg)
     // need to sync after each command complete of a prepared statement
-    if (this.isPreparedStatement) {
+    if (this.isPreparedStatement && !this._hasSentSync) {
+      this._hasSentSync = true
       con.sync()
     }
   }
@@ -109,7 +111,8 @@ class Query extends EventEmitter {
   // the backend will send an emptyQuery message but *not* a command complete message
   // execution on the connection will hang until the backend receives a sync message
   handleEmptyQuery(con) {
-    if (this.isPreparedStatement) {
+    if (this.isPreparedStatement && !this._hasSentSync) {
+      this._hasSentSync = true
       con.sync()
     }
   }
@@ -126,7 +129,13 @@ class Query extends EventEmitter {
 
   handleError(err, connection) {
     // need to sync after error during a prepared statement
-    if (this.isPreparedStatement) {
+    // in postgres 9.6 the backend sends both a command complete and error response
+    // to a query which has timed out on rare, random occasions.  If we send sync twice we will receive
+    // to 'readyForQuery' events.  I think this might be a bug in postgres 9.6, but I'm not sure...
+    // the docs here: https://www.postgresql.org/docs/9.6/protocol-flow.html#PROTOCOL-FLOW-EXT-QUERY
+    // say "Therefore, an Execute phase is always terminated by the appearance of exactly one of these messages: CommandComplete, EmptyQueryResponse (if the portal was created from an empty query string), ErrorResponse, or PortalSuspended."
+    if (this.isPreparedStatement && !this._hasSentSync) {
+      this._hasSentSync = true
       connection.sync()
     }
     if (this._canceledDueToError) {
diff --git a/packages/pg/test/integration/gh-issues/1105-tests.js b/packages/pg/test/integration/gh-issues/1105-tests.js
@@ -0,0 +1,20 @@
+const pg = require('../../../lib')
+const helper = require('../test-helper')
+const suite = new helper.Suite()
+
+suite.testAsync('timeout causing query crashes', async () => {
+  const client = new helper.Client()
+  await client.connect()
+  await client.query('CREATE TEMP TABLE foobar( name TEXT NOT NULL, id SERIAL)')
+  client.query('BEGIN')
+  await client.query("SET LOCAL statement_timeout TO '1ms'")
+  let count = 0
+  while (count++ < 5000) {
+    try {
+      await client.query('INSERT INTO foobar(name) VALUES ($1)', [Math.random() * 1000 + ''])
+    } catch (e) {
+      await client.query('ROLLBACK')
+    }
+  }
+  await client.end()
+})
diff --git a/packages/pg/test/integration/gh-issues/2085-tests.js b/packages/pg/test/integration/gh-issues/2085-tests.js
@@ -4,6 +4,10 @@ var assert = require('assert')
 
 const suite = new helper.Suite()
 
+if (process.env.PGTESTNOSSL) {
+  return
+}
+
 suite.testAsync('it should connect over ssl', async () => {
   const ssl = helper.args.native
     ? 'require'
