Close backendConn after use #22
Conversation
| fullMethodName := lowLevelServerStream.Method() | ||
| // We require that the director's returned context inherits from the serverStream.Context(). | ||
| outgoingCtx, backendConn, err := s.director(serverStream.Context(), fullMethodName) | ||
| defer backendConn.Close() |
There was a problem hiding this comment.
This should go through the director, IMO. Also this shouldn't occur unless the call succeeds, otherwise you can get a NPE panic since backendConn may be nil.
Need for backend cleanup was noted in #16, which I've implemented on a separate branch: https://github.com/vgough/grpc-proxy/blob/master/proxy/handler.go#L71
There was a problem hiding this comment.
Hi @vgough. Isn't it enough to just 'defer backendConn.Close()' after checking if director returned an error to prevent nil pointer dereference
There was a problem hiding this comment.
You're responding to a comment that's over 4 years old. I'm not using this anymore. Be sure to test your setup under load. Yes, at the very minimum there should be a nil check to avoid a NPE.
3 participants
Look like backend connection never get closed and we should after use it.