From 3875acf3922501ddbbbdbcd4b18e81ffeb70757f Mon Sep 17 00:00:00 2001 From: Otto van der Schaaf Date: Thu, 5 Mar 2015 13:30:44 +0100 Subject: [PATCH] native-fetcher: fixes - Native fetching broke in https://github.com/pagespeed/ngx_pagespeed/pull/925 Make changes so we can do early-stage fetches in the startup process to fix that. - Properly cancel out any pending fetches upon ShutDown() - Decline fetching after ShutDown(), decline fetching when Init() failed. --- src/ngx_pagespeed.cc | 3 --- src/ngx_rewrite_driver_factory.cc | 13 +------------ src/ngx_rewrite_driver_factory.h | 1 - src/ngx_url_async_fetcher.cc | 17 +++++++++++++++++ 4 files changed, 18 insertions(+), 16 deletions(-) diff --git a/src/ngx_pagespeed.cc b/src/ngx_pagespeed.cc index 7b978b637..f6cfbe9f9 100644 --- a/src/ngx_pagespeed.cc +++ b/src/ngx_pagespeed.cc @@ -3067,9 +3067,6 @@ ngx_int_t ps_init_child_process(ngx_cycle_t* cycle) { } } - if (!cfg_m->driver_factory->InitNgxUrlAsyncFetchers()) { - return NGX_ERROR; - } cfg_m->driver_factory->StartThreads(); return NGX_OK; } diff --git a/src/ngx_rewrite_driver_factory.cc b/src/ngx_rewrite_driver_factory.cc index 9bc055a3d..aeef4afc9 100644 --- a/src/ngx_rewrite_driver_factory.cc +++ b/src/ngx_rewrite_driver_factory.cc @@ -152,18 +152,6 @@ RewriteOptions* NgxRewriteDriverFactory::NewRewriteOptions() { return options; } -bool NgxRewriteDriverFactory::InitNgxUrlAsyncFetchers() { - log_ = ngx_cycle->log; - for (size_t i = 0; i < ngx_url_async_fetchers_.size(); ++i) { - // TODO(oschaaf): Can we pass the log from the server{} block here? - if (!ngx_url_async_fetchers_[i]->Init( - const_cast(ngx_cycle))) { - return false; - } - } - return true; -} - bool NgxRewriteDriverFactory::CheckResolver() { if (use_native_fetcher_ && resolver_ == NULL) { return false; @@ -221,6 +209,7 @@ void NgxRewriteDriverFactory::StartThreads() { } void NgxRewriteDriverFactory::LoggingInit(ngx_log_t* log) { + log_ = log; net_instaweb::log_message_handler::Install(log); if (install_crash_handler()) { NgxMessageHandler::InstallCrashHandler(log); diff --git a/src/ngx_rewrite_driver_factory.h b/src/ngx_rewrite_driver_factory.h index 957d14492..a35cb2532 100644 --- a/src/ngx_rewrite_driver_factory.h +++ b/src/ngx_rewrite_driver_factory.h @@ -62,7 +62,6 @@ class NgxRewriteDriverFactory : public SystemRewriteDriverFactory { // NgxRewriteOptions. virtual RewriteOptions* NewRewriteOptions(); virtual ServerContext* NewDecodingServerContext(); - bool InitNgxUrlAsyncFetchers(); // Check resolver configured or not. bool CheckResolver(); diff --git a/src/ngx_url_async_fetcher.cc b/src/ngx_url_async_fetcher.cc index 4a85cc0e5..b90101604 100644 --- a/src/ngx_url_async_fetcher.cc +++ b/src/ngx_url_async_fetcher.cc @@ -78,6 +78,12 @@ namespace net_instaweb { log_ = log; pool_ = NULL; resolver_ = resolver; + // If init fails, set shutdown_ so no fetches will be attempted. + if (!Init(const_cast(ngx_cycle))) { + shutdown_ = true; + message_handler_->Message( + kError, "NgxUrlAsyncFetcher failed to init, fetching disabled."); + } } NgxUrlAsyncFetcher::~NgxUrlAsyncFetcher() { @@ -178,6 +184,11 @@ namespace net_instaweb { void NgxUrlAsyncFetcher::ShutDown() { shutdown_ = true; if (!pending_fetches_.empty()) { + for (Pool::iterator p = pending_fetches_.begin(), + e = pending_fetches_.end(); p != e; p++) { + NgxFetch* fetch = *p; + fetch->CallbackDone(false); + } pending_fetches_.DeleteAll(); } @@ -201,6 +212,12 @@ namespace net_instaweb { void NgxUrlAsyncFetcher::Fetch(const GoogleString& url, MessageHandler* message_handler, AsyncFetch* async_fetch) { + // Don't accept new fetches when shut down. This flow is also entered when + // we did not initialize properly in ::Init(). + if (shutdown_) { + async_fetch->Done(false); + return; + } async_fetch = EnableInflation(async_fetch); NgxFetch* fetch = new NgxFetch(url, async_fetch, message_handler, log_);