Alanxtl commented on code in PR #3579:
URL: https://github.com/apache/dubbo-go/pull/3579#discussion_r3755516438
##########
protocol/jsonrpc/server.go:
##########
@@ -79,14 +79,20 @@ func NewServer() *Server {
}
func (s *Server) handlePkg(conn net.Conn) {
+ connectionCtx, connectionCancel :=
context.WithCancel(context.Background())
+ writeConn := &lockedConn{Conn: conn}
+ var requestWG sync.WaitGroup
defer func() {
if r := recover(); r != nil {
logger.Warnf("[Jsonrpc][Server] connection panic,
local=%v, remote=%v, err=%v, debug stack=%s",
conn.LocalAddr(), conn.RemoteAddr(), r,
string(debug.Stack()))
}
conn.Close()
+ requestWG.Wait()
}()
+ // Register this after the cleanup defer so LIFO ordering cancels
request contexts before Wait.
+ defer connectionCancel()
Review Comment:
`handlePkg` now starts a goroutine per parsed request. The `lockedConn`
wrapper prevents byte-level interleaving, but it does not preserve HTTP
response ordering. If a client sends multiple HTTP/1.1 requests on the same
connection and the first invocation is slower than the second, the second
response can be written first, which violates HTTP/1.1 pipelining semantics and
can make clients associate responses with the wrong requests.
Please keep per-connection response writes ordered, or avoid concurrent
request handling on a single connection while still using a connection-level
cancelable context for disconnect cancellation. A regression test with two
same-connection requests where request 1 blocks and request 2 returns
immediately would catch this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
