From 8309ddd486b078f6e78011a357f6208a5f20a4e8 Mon Sep 17 00:00:00 2001 From: Klaus Post Date: Thu, 2 Dec 2021 11:22:32 -0800 Subject: [PATCH] Fix panic (not fatal) on connection drops (#13811) Fix more regressions from #13597 with double closed channels. ``` panic: "POST /minio/storage/data/distxl-plain/s1/d2/v42/createfile?disk-id=c789f7e1-2b52-442a-b518-aa2dac03f3a1&file-path=f6161668-b939-4543-9873-91b9da4cdff6%2F5eafa986-a3bf-4b1c-8bc0-03a37de390a3%2Fpart.1&length=2621760&volume=.minio.sys%2Ftmp": send on closed channel goroutine 1977 [running]: runtime/debug.Stack() c:/go/src/runtime/debug/stack.go:24 +0x65 github.com/minio/minio/cmd.setCriticalErrorHandler.func1.1() d:/minio/minio/cmd/generic-handlers.go:468 +0x8e panic({0x2928860, 0x4fb17e0}) c:/go/src/runtime/panic.go:1038 +0x215 github.com/minio/minio/cmd.keepHTTPReqResponseAlive.func2({0x4fe4ea0, 0xc02737d8a0}) d:/minio/minio/cmd/storage-rest-server.go:818 +0x48 github.com/minio/minio/cmd.(*storageRESTServer).CreateFileHandler(0xc0015a8510, {0x50073e0, 0xc0273ec460}, 0xc029b9a400) d:/minio/minio/cmd/storage-rest-server.go:334 +0x1d2 net/http.HandlerFunc.ServeHTTP(...) c:/go/src/net/http/server.go:2046 github.com/minio/minio/cmd.httpTraceHdrs.func1({0x50073e0, 0xc0273ec460}, 0x0) d:/minio/minio/cmd/handler-utils.go:372 +0x53 net/http.HandlerFunc.ServeHTTP(0x5007380, {0x50073e0, 0xc0273ec460}, 0x10) c:/go/src/net/http/server.go:2046 +0x2f github.com/minio/minio/cmd.addCustomHeaders.func1({0x5007380, 0xc0273dcf00}, 0xc0273f7340) ``` Reverts but adds write checks. --- cmd/storage-rest-server.go | 73 ++++++++++++++++++++------------------ 1 file changed, 38 insertions(+), 35 deletions(-) diff --git a/cmd/storage-rest-server.go b/cmd/storage-rest-server.go index f79890716..56b3c82f6 100644 --- a/cmd/storage-rest-server.go +++ b/cmd/storage-rest-server.go @@ -760,51 +760,48 @@ func keepHTTPReqResponseAlive(w http.ResponseWriter, r *http.Request) (resp func doneCh := make(chan error) ctx := r.Context() go func() { - defer close(doneCh) + var canWrite = true + write := func(b []byte) { + if canWrite { + n, err := w.Write(b) + if err != nil || n != len(b) { + canWrite = false + } + } + } // Wait for body to be read. select { case <-ctx.Done(): - return case <-bodyDoneCh: - case err, ok := <-doneCh: - if !ok { - return - } + case err := <-doneCh: if err != nil { - _, werr := w.Write([]byte{1}) - if werr != nil { - return - } - w.Write([]byte(err.Error())) + write([]byte{1}) + write([]byte(err.Error())) } else { - w.Write([]byte{0}) + write([]byte{0}) } + close(doneCh) return } + defer close(doneCh) // Initiate ticker after body has been read. ticker := time.NewTicker(time.Second * 10) - defer ticker.Stop() for { select { case <-ticker.C: // Response not ready, write a filler byte. - if _, err := w.Write([]byte{32}); err != nil { - return - } - w.(http.Flusher).Flush() - case err, ok := <-doneCh: - if !ok { - return + write([]byte{32}) + if canWrite { + w.(http.Flusher).Flush() } + case err := <-doneCh: if err != nil { - _, werr := w.Write([]byte{1}) - if werr != nil { - return - } - w.Write([]byte(err.Error())) + write([]byte{1}) + write([]byte(err.Error())) } else { - w.Write([]byte{0}) + write([]byte{0}) } + ticker.Stop() return } } @@ -837,6 +834,15 @@ func keepHTTPReqResponseAlive(w http.ResponseWriter, r *http.Request) (resp func func keepHTTPResponseAlive(w http.ResponseWriter) func(error) { doneCh := make(chan error) go func() { + var canWrite = true + write := func(b []byte) { + if canWrite { + n, err := w.Write(b) + if err != nil || n != len(b) { + canWrite = false + } + } + } defer close(doneCh) ticker := time.NewTicker(time.Second * 10) defer ticker.Stop() @@ -844,19 +850,16 @@ func keepHTTPResponseAlive(w http.ResponseWriter) func(error) { select { case <-ticker.C: // Response not ready, write a filler byte. - if _, err := w.Write([]byte{32}); err != nil { - return + write([]byte{32}) + if canWrite { + w.(http.Flusher).Flush() } - w.(http.Flusher).Flush() case err := <-doneCh: if err != nil { - _, werr := w.Write([]byte{1}) - if werr != nil { - return - } - w.Write([]byte(err.Error())) + write([]byte{1}) + write([]byte(err.Error())) } else { - w.Write([]byte{0}) + write([]byte{0}) } return }