IPRO: Fix a race in processing writes() from NgxBaseFetch

For FetchInPlaceResource, NgxBaseFetch would send two bytes down its
pipe, one upon HeaderComplete() and one upon HandleDone(). We need
only one to resume processing on the nginx side.

There is a race between ps_connection_read_handler() and processing
of the byte send by NgxBaseFetch::HandleDone().
ps_connection_read_handler() clears the pipe when the request is
finalized, and also drains it on each event - so two writes could be
processed as one when lucky, masking the problem).

One concrete problem this solved for me was that SPDY + IPRO +
proxy_pass would segfault, hang, and/or pass on 5xx/404 responses
from IPRO lookup fetches to the browser, next to alerts about
r->count being zero in nginx's error.log

Might fix https://github.com/pagespeed/ngx_pagespeed/issues/788
Fixes https://github.com/pagespeed/ngx_pagespeed/issues/792
This commit is contained in:
Otto van der Schaaf
2014-09-11 23:14:41 +02:00
parent 622d088a3e
commit 43d1706e2f
3 changed files with 12 additions and 6 deletions
+9 -3
View File
@@ -39,7 +39,7 @@ NgxBaseFetch::NgxBaseFetch(ngx_http_request_t* r, int pipe_fd,
last_buf_sent_(false),
pipe_fd_(pipe_fd),
references_(2),
handle_error_(true),
ipro_lookup_(false),
preserve_caching_headers_(preserve_caching_headers) {
if (pthread_mutex_init(&mutex_, NULL)) CHECK(0);
}
@@ -137,14 +137,20 @@ void NgxBaseFetch::HandleHeadersComplete() {
int status_code = response_headers()->status_code();
bool status_ok = (status_code != 0) && (status_code < 400);
if (status_ok || handle_error_) {
if (!ipro_lookup_ || status_ok) {
// If this is a 404 response we need to count it in the stats.
if (response_headers()->status_code() == HttpStatus::kNotFound) {
server_context_->rewrite_stats()->resource_404_count()->Add(1);
}
}
RequestCollection(); // Headers available.
// For the IPRO lookup, supress notification of the nginx side here.
// If we send both this event and the one from done, nasty stuff will happen
// if we loose the race with with the nginx side destructing this base fetch
// instance (and thereby clearing the byte and its pending extraneous event.
if (!ipro_lookup_) {
RequestCollection(); // Headers available.
}
}
bool NgxBaseFetch::HandleFlush(MessageHandler* handler) {