Skip to content

Commit e359c76

Browse files
authored
fix huge memory leak in RPC system (#2510)
always remove from reqmap in Finalize even if done flag is already set.
1 parent 984ed46 commit e359c76

2 files changed

Lines changed: 22 additions & 18 deletions

File tree

package-lock.json

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

pkg/wshutil/wshrpc.go

Lines changed: 20 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -269,25 +269,26 @@ func (w *WshRpc) cancelRequest(reqId string) {
269269
}
270270

271271
func (w *WshRpc) handleRequest(req *RpcMessage) {
272-
pprof.Do(context.Background(), pprof.Labels("rpc", req.Command), func(ctx context.Context) {
273-
w.handleRequestInternal(req)
272+
pprof.Do(context.Background(), pprof.Labels("rpc", req.Command), func(pprofCtx context.Context) {
273+
w.handleRequestInternal(req, pprofCtx)
274274
})
275275
}
276276

277-
func (w *WshRpc) handleRequestInternal(req *RpcMessage) {
278-
// events first
277+
func (w *WshRpc) handleEventRecv(req *RpcMessage) {
278+
if req.Data == nil {
279+
return
280+
}
281+
var waveEvent wps.WaveEvent
282+
err := utilfn.ReUnmarshal(&waveEvent, req.Data)
283+
if err != nil {
284+
return
285+
}
286+
w.EventListener.RecvEvent(&waveEvent)
287+
}
288+
289+
func (w *WshRpc) handleRequestInternal(req *RpcMessage, pprofCtx context.Context) {
279290
if req.Command == wshrpc.Command_EventRecv {
280-
if req.Data == nil {
281-
// invalid
282-
return
283-
}
284-
var waveEvent wps.WaveEvent
285-
err := utilfn.ReUnmarshal(&waveEvent, req.Data)
286-
if err != nil {
287-
// invalid
288-
return
289-
}
290-
w.EventListener.RecvEvent(&waveEvent)
291+
w.handleEventRecv(req)
291292
return
292293
}
293294

@@ -670,12 +671,15 @@ func (handler *RpcResponseHandler) close() {
670671

671672
// if async, caller must call finalize
672673
func (handler *RpcResponseHandler) Finalize() {
674+
// Always unregister the handler from the map, even if already done
675+
if handler.reqId != "" {
676+
handler.w.unregisterResponseHandler(handler.reqId)
677+
}
673678
if handler.reqId == "" || handler.done.Load() {
674679
return
675680
}
676681
handler.SendResponse(nil, true)
677682
handler.close()
678-
handler.w.unregisterResponseHandler(handler.reqId)
679683
}
680684

681685
func (handler *RpcResponseHandler) IsDone() bool {

0 commit comments

Comments
 (0)