[agent] Fix a race condition when writing a response to a cancelled request. #135
Conversation
…equest. When a request is cancelled prior to the response being written, we might still get a response from the backend and try to write it. Previously, if that happened the write of the response body would hang forever, waiting for the response to be serialized to the no-longer extant proxy request. With this change, that no longer happens because we catch this scenario in the `WriteHeader` method and close the body reader... causing any subsequent writes to immediately fail.
| @@ -543,7 +548,7 @@ func (w *streamingResponseWriter) CloseWithError(err error) error { | |||
| func NewResponseForwarder(client *http.Client, proxyHost, backendID, requestID string, r *http.Request, metricHandler *metrics.MetricHandler) (ResponseWriteCloser, error) { | |||
| // Construct a streaming response writer so we can process the written response | |||
| // in separate goroutines as it is written. | |||
| respChan := make(chan *http.Response, 1) | |||
| respChan := make(chan *http.Response) | |||
There was a problem hiding this comment.
Just to clarify, is this change needed for the fix to work, or is it more of a defensive change?
There was a problem hiding this comment.
This is needed for the fix to work.
The issue was that on one end the response was being buffered, but on the other end it was never being read and consumed.
The fact that it wasn't consumed (written to the proxy request) meant that the response body was never being closed, and thus subsequent writes to the response body would block indefinitely.
To fix this, we have to make the goroutine reading the response and the one writing the response be synchronized through this channel.
NOTE: This reasoning is subtle, but no one has to remember it; if anyone tries to change this in the future the newly changed test will fail.
When a request is cancelled prior to the response being written, we might still get a response from the backend and try to write it.
Previously, if that happened the write of the response body would hang forever, waiting for the response to be serialized to the no-longer extant proxy request.
With this change, that no longer happens because we catch this scenario in the
WriteHeadermethod and close the body reader... causing any subsequent writes to immediately fail.