Aias00 opened a new issue, #3546:
URL: https://github.com/apache/dubbo-go/issues/3546
### Problem
`protocol/jsonrpc/server.go` `handlePkg` runs for the lifetime of one
keep-alive TCP connection and serves many HTTP requests in a `for` loop. Each
request carrying a `Timeout` header creates a `context.WithTimeout` whose
`cancel` is `defer`red **inside the loop**:
```go
// protocol/jsonrpc/server.go handlePkg, per request in the for-loop
if len(reqHeader["Timeout"]) > 0 {
timeout, err := time.ParseDuration(reqHeader["Timeout"])
if err == nil {
httpTimeout = timeout
var cancel context.CancelFunc
ctx, cancel = context.WithTimeout(ctx, httpTimeout)
defer cancel() // <-- defer inside a loop, in a long-lived fn
}
delete(reqHeader, "Timeout")
}
```
`defer` only runs when `handlePkg` returns (i.e. when the connection
closes), so each keep-alive request with a `Timeout` header accumulates one
deferred `cancel` + its retained context (and the canceler/timer) in the
connection goroutine until the connection tears down.
### Current behavior
A client opens a persistent/keep-alive connection and sends many requests
each with a `Timeout` header. Each request accumulates one deferred `cancel` +
retained context in the connection goroutine → unbounded growth of the defer
chain and retained context objects → slow memory growth / eventual OOM under
sustained keep-alive traffic. The `ctx` is also not cancelled promptly after
`serveRequest` returns (the timer holds resources until it fires or the
connection closes).
### Expected behavior
The timeout context should be cancelled promptly after `serveRequest`
returns for that request, with no accumulation across the keep-alive connection.
### Suggested approach
Replace `defer cancel()` (inside the loop) with an explicit `cancel()` call
after `serveRequest` returns:
```go
var cancel context.CancelFunc
if len(reqHeader["Timeout"]) > 0 {
timeout, err := time.ParseDuration(reqHeader["Timeout"])
if err == nil {
httpTimeout = timeout
ctx, cancel = context.WithTimeout(ctx, httpTimeout)
}
delete(reqHeader, "Timeout")
}
setTimeout(conn, httpTimeout)
serveErr := serveRequest(ctx, reqHeader, reqBody, conn)
if cancel != nil { cancel() }
if serveErr != nil { ... return }
```
### Acceptance criteria
- [ ] No `defer` accumulates per request in the `handlePkg` loop.
- [ ] The timeout context is cancelled after `serveRequest` returns.
- [ ] Existing jsonrpc tests remain green.
--
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]
