Merge pull request #794 from pagespeed/oschaaf-ipro-race

IPRO: Fix a race in processing writes() from NgxBaseFetch
This commit is contained in:
Otto van der Schaaf
2014-09-12 16:33:29 +02:00
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), last_buf_sent_(false),
pipe_fd_(pipe_fd), pipe_fd_(pipe_fd),
references_(2), references_(2),
handle_error_(true), ipro_lookup_(false),
preserve_caching_headers_(preserve_caching_headers) { preserve_caching_headers_(preserve_caching_headers) {
if (pthread_mutex_init(&mutex_, NULL)) CHECK(0); if (pthread_mutex_init(&mutex_, NULL)) CHECK(0);
} }
@@ -137,14 +137,20 @@ void NgxBaseFetch::HandleHeadersComplete() {
int status_code = response_headers()->status_code(); int status_code = response_headers()->status_code();
bool status_ok = (status_code != 0) && (status_code < 400); 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 this is a 404 response we need to count it in the stats.
if (response_headers()->status_code() == HttpStatus::kNotFound) { if (response_headers()->status_code() == HttpStatus::kNotFound) {
server_context_->rewrite_stats()->resource_404_count()->Add(1); 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) { bool NgxBaseFetch::HandleFlush(MessageHandler* handler) {
+2 -2
View File
@@ -79,7 +79,7 @@ class NgxBaseFetch : public AsyncFetch {
// Called by nginx when it's done with us. // Called by nginx when it's done with us.
void Release(); void Release();
void set_handle_error(bool x) { handle_error_ = x; } void set_ipro_lookup(bool x) { ipro_lookup_ = x; }
private: private:
virtual bool HandleWrite(const StringPiece& sp, MessageHandler* handler); virtual bool HandleWrite(const StringPiece& sp, MessageHandler* handler);
@@ -117,7 +117,7 @@ class NgxBaseFetch : public AsyncFetch {
// decremented once when Done() is called and once when Release() is called. // decremented once when Done() is called and once when Release() is called.
int references_; int references_;
pthread_mutex_t mutex_; pthread_mutex_t mutex_;
bool handle_error_; bool ipro_lookup_;
PreserveCachingHeaders preserve_caching_headers_; PreserveCachingHeaders preserve_caching_headers_;
DISALLOW_COPY_AND_ASSIGN(NgxBaseFetch); DISALLOW_COPY_AND_ASSIGN(NgxBaseFetch);
+1 -1
View File
@@ -2022,7 +2022,7 @@ ngx_int_t ps_resource_handler(ngx_http_request_t* r,
url_string.c_str()); url_string.c_str());
ctx->in_place = true; ctx->in_place = true;
ctx->base_fetch->set_handle_error(false); ctx->base_fetch->set_ipro_lookup(true);
ctx->driver->FetchInPlaceResource( ctx->driver->FetchInPlaceResource(
url, false /* proxy_mode */, ctx->base_fetch); url, false /* proxy_mode */, ctx->base_fetch);