From 9c678e108c4ef73187d16bd7b6fae8cd71fe9895 Mon Sep 17 00:00:00 2001 From: "Brian M. Carlson" Date: Wed, 7 Oct 2020 14:50:12 -0500 Subject: [PATCH] Fix double-sync crash on postgres 9.x --- packages/pg/lib/query.js | 15 +++++++++++--- .../test/integration/gh-issues/1105-tests.js | 20 +++++++++++++++++++ .../test/integration/gh-issues/2085-tests.js | 4 ++++ 3 files changed, 36 insertions(+), 3 deletions(-) create mode 100644 packages/pg/test/integration/gh-issues/1105-tests.js diff --git a/packages/pg/lib/query.js b/packages/pg/lib/query.js index 9cd0dab1..26d0aa61 100644 --- 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 new file mode 100644 index 00000000..2a36d699 --- /dev/null +++ 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 index 23fd71d0..d65b5fdc 100644 --- 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'