From 589eb9dcb60c45ccec34e36223f393bb97ede0af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Martins?= Date: Mon, 18 Mar 2019 10:35:07 +0100 Subject: [PATCH] transport: do not close channel that can lean to panic MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `Write` can be called concurrently, for which it calls the `do` function. As `WriteStatus` can close the `ht.writes` in parallel as well the `Write` will try to write into the `ht.writes` in the `do` function, this can lead into a panic. As there is no real usability on closing this channel we can simply leave it to the garbage collector so we can avoid panic during an execution. Signed-off-by: André Martins --- internal/transport/handler_server.go | 16 +++------------- 1 file changed, 3 insertions(+), 13 deletions(-) diff --git a/internal/transport/handler_server.go b/internal/transport/handler_server.go index 73b41ea7e0b..695490c4798 100644 --- a/internal/transport/handler_server.go +++ b/internal/transport/handler_server.go @@ -176,17 +176,11 @@ func (a strAddr) String() string { return string(a) } // do runs fn in the ServeHTTP goroutine. func (ht *serverHandlerTransport) do(fn func()) error { - // Avoid a panic writing to closed channel. Imperfect but maybe good enough. select { case <-ht.closedCh: return ErrConnClosing - default: - select { - case ht.writes <- fn: - return nil - case <-ht.closedCh: - return ErrConnClosing - } + case ht.writes <- fn: + return nil } } @@ -237,7 +231,6 @@ func (ht *serverHandlerTransport) WriteStatus(s *Stream, st *status.Status) erro if ht.stats != nil { ht.stats.HandleRPC(s.Context(), &stats.OutTrailer{}) } - close(ht.writes) } ht.Close() return err @@ -407,10 +400,7 @@ func (ht *serverHandlerTransport) HandleStreams(startStream func(*Stream), trace func (ht *serverHandlerTransport) runStream() { for { select { - case fn, ok := <-ht.writes: - if !ok { - return - } + case fn := <-ht.writes: fn() case <-ht.closedCh: return