fix client deadlock on server death during writes
Fixes #234
Similar to #181 (which was about read methods), the client deadlocks on
server drop on write methods (Write/ReadFrom). This is another case of
broadcastErr() and dispatchRequest() deadlocking.
The fix is to bump up the channel used to communicate about inflight
packets to maxConcurrentRequests+1 (+1 is new). It seems that it can
have the full set of packets going and hit the error (triggering
broadcastErr) yet it still tries to call dispatchRequest again. More
details in that ticket.
I'm also bumping up the chan buffer in the read methods to keep them
consistent. I can't reproduce the problem, but it looks like it should
have it. So it might just be a much harder race to trigger.
This also includes 2 tests which reprocuded the issue for ReadFrom and
Write.
Change-Id: Icb6cefc682da28b8bd495cc8fddd65b9a37eb448
diff --git a/client.go b/client.go
index c3012c0..d0ccf13 100644
--- a/client.go
+++ b/client.go
@@ -675,7 +675,7 @@
offset := f.offset
// maxConcurrentRequests buffer to deal with broadcastErr() floods
// also must have a buffer of max value of (desiredInFlight - inFlight)
- ch := make(chan result, maxConcurrentRequests)
+ ch := make(chan result, maxConcurrentRequests+1)
type inflightRead struct {
b []byte
offset uint64
@@ -771,7 +771,7 @@
writeOffset := offset
fileSize := uint64(fi.Size())
// see comment on same line in Read() above
- ch := make(chan result, maxConcurrentRequests)
+ ch := make(chan result, maxConcurrentRequests+1)
type inflightRead struct {
b []byte
offset uint64
@@ -914,7 +914,7 @@
desiredInFlight := 1
offset := f.offset
// see comment on same line in Read() above
- ch := make(chan result, maxConcurrentRequests)
+ ch := make(chan result, maxConcurrentRequests+1)
var firstErr error
written := len(b)
for len(b) > 0 || inFlight > 0 {
@@ -975,7 +975,7 @@
desiredInFlight := 1
offset := f.offset
// see comment on same line in Read() above
- ch := make(chan result, maxConcurrentRequests)
+ ch := make(chan result, maxConcurrentRequests+1)
var firstErr error
read := int64(0)
b := make([]byte, f.c.maxPacket)
diff --git a/client_integration_test.go b/client_integration_test.go
index ef73a08..40dca8a 100644
--- a/client_integration_test.go
+++ b/client_integration_test.go
@@ -1740,6 +1740,68 @@
}
}
+// sftp/issue/234 - abrupt shutdown during ReadFrom hangs client
+func TestServerRoughDisconnect3(t *testing.T) {
+ if *testServerImpl {
+ t.Skipf("skipping with -testserver")
+ }
+ sftp, cmd := testClient(t, READWRITE, NO_DELAY)
+ defer cmd.Wait()
+ defer sftp.Close()
+
+ rf, err := sftp.OpenFile("/dev/null", os.O_RDWR)
+ if err != nil {
+ t.Fatal(err)
+ }
+ defer rf.Close()
+ lf, err := os.Open("/dev/zero")
+ if err != nil {
+ t.Fatal(err)
+ }
+ defer lf.Close()
+ go func() {
+ time.Sleep(10 * time.Millisecond)
+ cmd.Process.Kill()
+ }()
+
+ io.Copy(rf, lf)
+}
+
+// sftp/issue/234 - also affected Write
+func TestServerRoughDisconnect4(t *testing.T) {
+ if *testServerImpl {
+ t.Skipf("skipping with -testserver")
+ }
+ sftp, cmd := testClient(t, READWRITE, NO_DELAY)
+ defer cmd.Wait()
+ defer sftp.Close()
+
+ rf, err := sftp.OpenFile("/dev/null", os.O_RDWR)
+ if err != nil {
+ t.Fatal(err)
+ }
+ defer rf.Close()
+ lf, err := os.Open("/dev/zero")
+ if err != nil {
+ t.Fatal(err)
+ }
+ defer lf.Close()
+ go func() {
+ time.Sleep(10 * time.Millisecond)
+ cmd.Process.Kill()
+ }()
+ b := make([]byte, 32768*200)
+ lf.Read(b)
+ for {
+ _, err = rf.Write(b)
+ if err != nil {
+ break
+ }
+ }
+
+ io.Copy(rf, lf)
+}
+
// sftp/issue/26 writing to a read only file caused client to loop.
func TestClientWriteToROFile(t *testing.T) {
sftp, cmd := testClient(t, READWRITE, NO_DELAY)
