lib/model, lib/testutils: Test closing a connection on folder restart (#5707)

lib/model/fakeconns_test.go

@@ -13,6 +13,7 @@ import (
 	"sync"
 	"time"
 
+	"github.com/syncthing/syncthing/lib/connections"
 	"github.com/syncthing/syncthing/lib/protocol"
 	"github.com/syncthing/syncthing/lib/scanner"
 )
@@ -23,6 +24,7 @@ type downloadProgressMessage struct {
 }
 
 type fakeConnection struct {
+	fakeUnderlyingConn
 	id                       protocol.DeviceID
 	downloadProgressMessages []downloadProgressMessage
 	closed                   bool
@@ -58,10 +60,6 @@ func (f *fakeConnection) Name() string {
 	return ""
 }
 
-func (f *fakeConnection) String() string {
-	return ""
-}
-
 func (f *fakeConnection) Option(string) string {
 	return ""
 }
@@ -111,26 +109,6 @@ func (f *fakeConnection) Statistics() protocol.Statistics {
 	return protocol.Statistics{}
 }
 
-func (f *fakeConnection) RemoteAddr() net.Addr {
-	return &fakeAddr{}
-}
-
-func (f *fakeConnection) Type() string {
-	return "fake"
-}
-
-func (f *fakeConnection) Crypto() string {
-	return "fake"
-}
-
-func (f *fakeConnection) Transport() string {
-	return "fake"
-}
-
-func (f *fakeConnection) Priority() int {
-	return 9000
-}
-
 func (f *fakeConnection) DownloadProgress(folder string, updates []protocol.FileDownloadProgressUpdate) {
 	f.downloadProgressMessages = append(f.downloadProgressMessages, downloadProgressMessage{
 		folder:  folder,
@@ -235,6 +213,43 @@ func addFakeConn(m *model, dev protocol.DeviceID) *fakeConnection {
 	return fc
 }
 
+type fakeProtoConn struct {
+	protocol.Connection
+	fakeUnderlyingConn
+}
+
+func newFakeProtoConn(protoConn protocol.Connection) connections.Connection {
+	return &fakeProtoConn{Connection: protoConn}
+}
+
+// fakeUnderlyingConn implements the methods of connections.Connection that are
+// not implemented by protocol.Connection
+type fakeUnderlyingConn struct{}
+
+func (f *fakeUnderlyingConn) RemoteAddr() net.Addr {
+	return &fakeAddr{}
+}
+
+func (f *fakeUnderlyingConn) Type() string {
+	return "fake"
+}
+
+func (f *fakeUnderlyingConn) Crypto() string {
+	return "fake"
+}
+
+func (f *fakeUnderlyingConn) Transport() string {
+	return "fake"
+}
+
+func (f *fakeUnderlyingConn) Priority() int {
+	return 9000
+}
+
+func (f *fakeUnderlyingConn) String() string {
+	return ""
+}
+
 type fakeAddr struct{}
 
 func (fakeAddr) Network() string {

lib/model/model_test.go

@@ -31,6 +31,7 @@ import (
 	"github.com/syncthing/syncthing/lib/osutil"
 	"github.com/syncthing/syncthing/lib/protocol"
 	srand "github.com/syncthing/syncthing/lib/rand"
+	"github.com/syncthing/syncthing/lib/testutils"
 	"github.com/syncthing/syncthing/lib/versioner"
 )
 
@@ -3381,3 +3382,38 @@ func TestSanitizePath(t *testing.T) {
 		}
 	}
 }
+
+// TestConnCloseOnRestart checks that there is no deadlock when calling Close
+// on a protocol connection that has a blocking reader (blocking writer can't
+// be done as the test requires clusterconfigs to go through).
+func TestConnCloseOnRestart(t *testing.T) {
+	w, fcfg := tmpDefaultWrapper()
+	m := setupModel(w)
+	defer func() {
+		m.Stop()
+		m.db.Close()
+		os.RemoveAll(fcfg.Filesystem().URI())
+		os.Remove(w.ConfigPath())
+	}()
+
+	br := &testutils.BlockingRW{}
+	nw := &testutils.NoopRW{}
+	m.AddConnection(newFakeProtoConn(protocol.NewConnection(device1, br, nw, m, "testConn", protocol.CompressNever)), protocol.HelloResult{})
+
+	newFcfg := fcfg.Copy()
+	newFcfg.Paused = true
+	done := make(chan struct{})
+	go func() {
+		m.RestartFolder(fcfg, newFcfg)
+		close(done)
+	}()
+	select {
+	case <-done:
+	case <-time.After(5 * time.Second):
+		t.Fatal("Timed out before folder restart returned")
+	}
+	m.pmut.RLock()
+	if len(m.conn) != 0 {
+		t.Errorf("Conn wasn't removed on restart (len(m.conn) == %v)", len(m.conn))
+	}
+}

lib/testutils/testutils.go

@@ -0,0 +1,31 @@
+// Copyright (C) 2019 The Syncthing Authors.
+//
+// This Source Code Form is subject to the terms of the Mozilla Public
+// License, v. 2.0. If a copy of the MPL was not distributed with this file,
+// You can obtain one at https://mozilla.org/MPL/2.0/.
+
+package testutils
+
+// BlockingRW implements io.Reader and Writer but never returns when called
+type BlockingRW struct{ nilChan chan struct{} }
+
+func (rw *BlockingRW) Read(p []byte) (n int, err error) {
+	<-rw.nilChan
+	return
+}
+
+func (rw *BlockingRW) Write(p []byte) (n int, err error) {
+	<-rw.nilChan
+	return
+}
+
+// NoopRW implements io.Reader and Writer but never returns when called
+type NoopRW struct{}
+
+func (rw *NoopRW) Read(p []byte) (n int, err error) {
+	return len(p), nil
+}
+
+func (rw *NoopRW) Write(p []byte) (n int, err error) {
+	return len(p), nil
+}