From 02752af86233849746c638f3e8cbbd78cce46878 Mon Sep 17 00:00:00 2001
From: Simon Frei <freisim93@gmail.com>
Date: Fri, 14 Jun 2019 19:04:41 +0200
Subject: [PATCH] lib/protocol: Don't block on Close (fixes #5794) (#5795)

---
 lib/protocol/common_test.go   |  4 ++++
 lib/protocol/protocol.go      |  6 +++++-
 lib/protocol/protocol_test.go | 19 +++++++++++++++++++
 3 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/lib/protocol/common_test.go b/lib/protocol/common_test.go
index 4d51a3d0e..1a15bcccf 100644
--- a/lib/protocol/common_test.go
+++ b/lib/protocol/common_test.go
@@ -14,6 +14,7 @@ type TestModel struct {
 	weakHash      uint32
 	fromTemporary bool
 	indexFn       func(DeviceID, string, []FileInfo)
+	ccFn          func(DeviceID, ClusterConfig)
 	closedCh      chan struct{}
 	closedErr     error
 }
@@ -52,6 +53,9 @@ func (t *TestModel) Closed(conn Connection, err error) {
 }
 
 func (t *TestModel) ClusterConfig(deviceID DeviceID, config ClusterConfig) {
+	if t.ccFn != nil {
+		t.ccFn(deviceID, config)
+	}
 }
 
 func (t *TestModel) DownloadProgress(DeviceID, string, []FileDownloadProgressUpdate) {
diff --git a/lib/protocol/protocol.go b/lib/protocol/protocol.go
index a081ec74f..66dbb54ce 100644
--- a/lib/protocol/protocol.go
+++ b/lib/protocol/protocol.go
@@ -882,7 +882,11 @@ func (c *rawConnection) Close(err error) {
 		}
 	})
 
-	c.internalClose(err)
+	// Close might be called from a method that is called from within
+	// dispatcherLoop, resulting in a deadlock.
+	// The sending above must happen before spawning the routine, to prevent
+	// the underlying connection from terminating before sending the close msg.
+	go c.internalClose(err)
 }
 
 // internalClose is called if there is an unexpected error during normal operation.
diff --git a/lib/protocol/protocol_test.go b/lib/protocol/protocol_test.go
index cba8dd3cf..a04c1f466 100644
--- a/lib/protocol/protocol_test.go
+++ b/lib/protocol/protocol_test.go
@@ -813,3 +813,22 @@ func TestClusterConfigAfterClose(t *testing.T) {
 		t.Fatal("timed out before Cluster Config returned")
 	}
 }
+
+func TestDispatcherToCloseDeadlock(t *testing.T) {
+	// Verify that we don't deadlock when calling Close() from within one of
+	// the model callbacks (ClusterConfig).
+	m := newTestModel()
+	c := NewConnection(c0ID, &testutils.BlockingRW{}, &testutils.NoopRW{}, m, "name", CompressAlways).(wireFormatConnection).Connection.(*rawConnection)
+	m.ccFn = func(devID DeviceID, cc ClusterConfig) {
+		c.Close(errManual)
+	}
+	c.Start()
+
+	c.inbox <- &ClusterConfig{}
+
+	select {
+	case <-c.dispatcherLoopStopped:
+	case <-time.After(time.Second):
+		t.Fatal("timed out before dispatcher loop terminated")
+	}
+}