ListenerConn: Fix deadlock when connection is lost while a query is q…

…ueued

Because the two goroutines acquire the two exclusive locks in different
orders, they would deadlock with each other for no reason.  Fix by
swapping the order of acquisition in acquireSenderLock.  If l.err is set
nobody should be holding senderLock for a long time anyway, so undesired
practical effects of this change should be very minimal.

notify.go

@@ -87,12 +87,16 @@ func NewListenerConn(name string, notificationChan chan<- *Notification) (*Liste
 // Returns an error if an unrecoverable error has occurred and the ListenerConn
 // should be abandoned.
 func (l *ListenerConn) acquireSenderLock() error {
+	// we must acquire senderLock first to avoid deadlocks; see ExecSimpleQuery
+	l.senderLock.Lock()
+
 	l.connectionLock.Lock()
-	defer l.connectionLock.Unlock()
-	if l.err != nil {
-		return l.err
+	err := l.err
+	l.connectionLock.Unlock()
+	if err != nil {
+		l.senderLock.Unlock()
+		return err
 	}
-	l.senderLock.Lock()
 	return nil
 }
 

notify_test.go

@@ -5,6 +5,9 @@ import (
 	"fmt"
 	"io"
 	"os"
+	"runtime"
+	"sync"
+	"sync/atomic"
 	"testing"
 	"time"
 )
@@ -210,6 +213,38 @@ func TestConnPing(t *testing.T) {
 	}
 }
 
+// Test for deadlock where a query fails while another one is queued
+func TestConnExecDeadlock(t *testing.T) {
+	l, _ := newTestListenerConn(t)
+	defer l.Close()
+
+	var wg sync.WaitGroup
+	wg.Add(2)
+
+	go func() {
+		l.ExecSimpleQuery("SELECT pg_sleep(60)")
+		wg.Done()
+	}()
+	go func() {
+		l.ExecSimpleQuery("SELECT 1")
+		wg.Done()
+	}()
+	// give the two goroutines some time to get into position
+	runtime.Gosched()
+	// simulate network connection failure
+	l.cn.c.Close()
+
+	var done int32 = 0
+	go func() {
+		time.Sleep(10 * time.Second)
+		if atomic.LoadInt32(&done) != 1 {
+			panic("timed out")
+		}
+	}()
+	wg.Wait()
+	atomic.StoreInt32(&done, 1)
+}
+
 func TestNotifyExtra(t *testing.T) {
 	db := openTestConn(t)
 	defer db.Close()