grpcproxy: only return ctx error in chan stream if recvc is empty #7345
Conversation
Codecov Report
@@ Coverage Diff @@
## master #7345 +/- ##
========================================
Coverage ? 63.9%
========================================
Files ? 233
Lines ? 20985
Branches ? 0
========================================
Hits ? 13411
Misses ? 6575
Partials ? 999
Continue to review full report at Codecov.
|
proxy/grpcproxy/chan_stream.go
Outdated
| select { | ||
| case msg, ok := <-s.recvc: | ||
| if !ok { | ||
| s.cancel() |
There was a problem hiding this comment.
This makes sense.
Did we just forget to cancel this context before? And now it fixes?
There was a problem hiding this comment.
That bit is just extra paranoia. The problem is from recvc being buffered and doing:
ch <- x
cancel()which leads to the select{} here having both <-s.recvc and <-s.ctx.Done() available for reading. Since go won't prioritize selects, it would randomly choose s.ctx.Done over s.recvc and return a context canceled error.
There was a problem hiding this comment.
Changing it back since it's not relevant to the fix
heyitsanthony
force-pushed
the
fix-stream-err
branch
from
96b769e
to
29a6fd6
Compare
|
LGTM |
Since select{} won't prioritize, ctx.Done() can sometimes override
a pending message on recvc. Loop if recvc has messages instead.
Fixes etcd-io#7340
Since select{} won't prioritize, ctx.Done() can sometimes override
a pending message on recvc. Loop if recvc has messages instead.
Fixes #7340