From 3b58955d6931c831de8c74202fb6d5e5d284dc35 Mon Sep 17 00:00:00 2001 From: Ian Denhardt Date: Fri, 13 Jan 2023 00:57:10 -0500 Subject: [PATCH] Fix #420 (without re-introducing #394) 420 was a regression introduced by our fix for 394; if we Return() before we fulfill(), this can result in out of order message delivery, since incoming calls can be made directly on clients in the result before all queued calls are drained. Previously, if we fulfilled() before Return(), we would get a data race as fillPayloadCapTable modified the message while pipelined calls read it. Having split Return(error) into PrepareReturn(error) and Return(), it is now safe to move just the latter after the call to fulfill(). --- server/server.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/server.go b/server/server.go index fcb4687f..0f29334e 100644 --- a/server/server.go +++ b/server/server.go @@ -217,12 +217,12 @@ func (srv *Server) handleCall(ctx context.Context, c *Call) { c.recv.ReleaseArgs() c.recv.Returner.PrepareReturn(err) - c.recv.Returner.Return() if err == nil { c.aq.fulfill(c.results) } else { c.aq.reject(err) } + c.recv.Returner.Return() c.recv.Returner.ReleaseResults() }