1.10.33.1 -> 1.10.33.2

@rfnx fixed #957 in acb89a, but this was accidentally merged to master
instead of trunk-tracking.  I checked for this sort of problem as part
of the 1.10 release, but missed this commit.  Restored.

Fixes #1054

.gitignore

@@ -1,4 +1,4 @@
 test/tmp
 psol/
 psol-*.tar.gz
-
+*.*.*.*.tar.gz

README.md

@@ -19,90 +19,15 @@ optimizations, see our <a href="http://ngxpagespeed.com">demonstration site</a>.
 
 ## How to build
 
-Because nginx does not support dynamic loading of modules, you need to compile
-nginx from source to add ngx_pagespeed. Alternatively, if you're using Tengine you can [install ngx_pagespeed without
-recompiling Tengine](https://github.com/pagespeed/ngx_pagespeed/wiki/Using-ngx_pagespeed-with-Tengine).
-
-1. Install dependencies:
-
-   ```bash
-   # These are for RedHat, CentOS, and Fedora.
-   $ sudo yum install gcc-c++ pcre-dev pcre-devel zlib-devel make
-
-   # These are for Debian. Ubuntu will be similar.
-   $ sudo apt-get install build-essential zlib1g-dev libpcre3 libpcre3-dev
-   ```
-
-2. Download ngx_pagespeed:
-
-   ```bash
-   $ cd ~
-   $ wget https://github.com/pagespeed/ngx_pagespeed/archive/v1.8.31.4-beta.zip
-   $ unzip v1.8.31.4-beta.zip # or unzip v1.8.31.4-beta
-   $ cd ngx_pagespeed-1.8.31.4-beta/
-   $ wget https://dl.google.com/dl/page-speed/psol/1.8.31.4.tar.gz
-   $ tar -xzvf 1.8.31.4.tar.gz # expands to psol/
-   ```
-
-3. Download and build nginx:
-
-   ```bash
-   $ cd ~
-   $ # check http://nginx.org/en/download.html for the latest version
-   $ wget http://nginx.org/download/nginx-1.6.0.tar.gz
-   $ tar -xvzf nginx-1.6.0.tar.gz
-   $ cd nginx-1.6.0/
-   $ ./configure --add-module=$HOME/ngx_pagespeed-1.8.31.4-beta
-   $ make
-   $ sudo make install
-   ```
-
-If this doesn't work see the [build
-troubleshooting](https://github.com/pagespeed/ngx_pagespeed/wiki/Build-Troubleshooting) page.
-
-This will use a binary PageSpeed Optimization Library, but it's also possible to
-[build PSOL from
-source](https://github.com/pagespeed/ngx_pagespeed/wiki/Building-PSOL-From-Source).
-
-Note: ngx_pagespeed currently doesn't support Windows or MacOS because the
-underlying PSOL library doesn't.
+Follow the steps on <a
+href="https://developers.google.com/speed/pagespeed/module/build_ngx_pagespeed_from_source">build
+ngx_pagespeed from source</a>.
 
 ## How to use
 
-In your `nginx.conf`, add to the main or server block:
-
-```nginx
-pagespeed on;
-pagespeed FileCachePath /var/ngx_pagespeed_cache;  # Use tmpfs for best results.
-```
-
-In every server block where pagespeed is enabled add:
-
-```apache
-#  Ensure requests for pagespeed optimized resources go to the pagespeed
-#  handler and no extraneous headers get set.
-location ~ "\.pagespeed\.([a-z]\.)?[a-z]{2}\.[^.]{10}\.[^.]+" { add_header "" ""; }
-location ~ "^/pagespeed_static/" { }
-location ~ "^/ngx_pagespeed_beacon$" { }
-```
-
-To confirm that the module is loaded, fetch a page and check that you see the
-`X-Page-Speed` header:
-
-```bash
-$ curl -I 'http://localhost:8050/some_page/' | grep X-Page-Speed
-X-Page-Speed: 1.8.31.4-...
-```
-
-Looking at the source of a few pages you should see various changes, such as
-urls being replaced with new ones like `yellow.css.pagespeed.ce.lzJ8VcVi1l.css`.
-
-For complete documentation, see [Using
-PageSpeed](https://developers.google.com/speed/pagespeed/module/using).
-
-There are extensive system tests which cover most of ngx_pagespeed's
-functionality.  Consider [testing your
-installation](https://github.com/pagespeed/ngx_pagespeed/wiki/Testing).
+Follow the steps on <a
+href="https://developers.google.com/speed/pagespeed/module/configuration">PageSpeed
+configuration</a>.
 
 For feedback, questions, and to follow
 the progress of the project:
@@ -111,20 +36,3 @@ the progress of the project:
   list](https://groups.google.com/forum/#!forum/ngx-pagespeed-discuss)
 - [ngx-pagespeed-announce mailing
   list](https://groups.google.com/forum/#!forum/ngx-pagespeed-announce)
-
-Note: The
-[canonicalize_javascript_libraries](https://developers.google.com/speed/pagespeed/module/filter-canonicalize-js)
-depends on `pagespeed_libraries.conf` which is distributed in Apache's format.
-To convert it to the Nginx format, run:
-
-```bash
-$ scripts/pagespeed_libraries_generator.sh > ~/pagespeed_libraries.conf
-$ sudo mv ~/pagespeed_libraries.conf /etc/nginx/
-```
-
-And then include it in your Nginx configuration by reference:
-
-```nginx
-include pagespeed_libraries.conf;
-pagespeed EnableFilters canonicalize_javascript_libraries;
-```

config

@@ -17,6 +17,8 @@
 #   PSOL_BINARY: absolute path to pagespeed_automatic.a
 
 mod_pagespeed_dir="${MOD_PAGESPEED_DIR:-unset}"
+position_aux="${POSITION_AUX:-unset}"
+
 if [ "$mod_pagespeed_dir" = "unset" ] ; then
   mod_pagespeed_dir="$ngx_addon_dir/psol/include"
   build_from_source=false
@@ -27,8 +29,8 @@ if [ "$mod_pagespeed_dir" = "unset" ] ; then
     echo "   You need to separately download the pagespeed library:"
     echo ""
     echo "     $ cd /path/to/ngx_pagespeed"
-    echo "     $ wget https://dl.google.com/dl/page-speed/psol/1.8.31.4.tar.gz"
-    echo "     $ tar -xzvf 1.8.31.4.tar.gz # expands to psol/"
+    echo "     $ wget https://dl.google.com/dl/page-speed/psol/1.10.33.2.tar.gz"
+    echo "     $ tar -xzvf 1.10.33.2.tar.gz # expands to psol/"
     echo ""
     echo "   Or see the installation instructions:"
     echo "     https://github.com/pagespeed/ngx_pagespeed#how-to-build"
@@ -39,20 +41,6 @@ else
   build_from_source=true
 fi
 
-echo "mod_pagespeed_dir=$mod_pagespeed_dir"
-echo "build_from_source=$build_from_source"
-
-ngx_feature="psol"
-ngx_feature_name=""
-ngx_feature_run=no
-ngx_feature_incs="
-#include \"net/instaweb/htmlparse/public/html_parse.h\"
-#include \"net/instaweb/htmlparse/public/html_writer_filter.h\"
-#include \"net/instaweb/util/public/string.h\"
-#include \"net/instaweb/util/public/string_writer.h\"
-#include \"net/instaweb/util/public/null_message_handler.h\"
-"
-
 os_name='unknown_os'
 arch_name='unknown_arch'
 uname_os=`uname`
@@ -83,6 +71,20 @@ else
   buildtype=Release
 fi
 
+# If the compiler is gcc, we want to use g++ to link, if at all possible,
+# so that -static-libstdc++ works.
+# Annoyingly, the feature test doesn't even use $LINK for linking, so that
+# needs an explicit -lstdc++
+pagespeed_libs=
+ps_maybe_gpp_base=`basename $CC| sed s/gcc/g++/`
+ps_maybe_gpp="`dirname $CC`/$ps_maybe_gpp_base"
+if [ -n "$NGX_GCC_VER" -a \( -x "$ps_maybe_gpp" \) ]; then
+  LINK=$ps_maybe_gpp
+  NGX_TEST_LD_OPT="$NGX_TEST_LD_OPT -lstdc++"
+else
+  pagespeed_libs="-lstdc++"
+fi
+
 # The compiler needs to know that __sync_add_and_fetch_4 is ok,
 # and this requires an instruction that didn't exist on i586 or i386.
 if [ "$uname_arch" = "i686" ]; then
@@ -91,6 +93,13 @@ fi
 
 CFLAGS="$CFLAGS $FLAG_MARCH"
 
+# For now, standardize on gcc-4.x ABI --- if we don't set this, people building
+# with new gcc defaulting to gcc-5 C++11 ABI will have build trouble linking
+# to our libpsol.a
+# See https://gcc.gnu.org/onlinedocs/libstdc++/manual/using_dual_abi.html
+CFLAGS="$CFLAGS -D_GLIBCXX_USE_CXX11_ABI=0"
+CC_TEST_FLAGS="$CC_TEST_FLAGS -D_GLIBCXX_USE_CXX11_ABI=0"
+
 case "$NGX_GCC_VER" in
   4.8*)
     # On GCC 4.8 and above, -Wall enables -Wunused-local-typedefs.  This breaks
@@ -113,6 +122,31 @@ if [ "$WNO_ERROR" = "YES" ]; then
     CFLAGS="$CFLAGS -Wno-error"
 fi
 
+psol_binary="${PSOL_BINARY:-unset}"
+if [ "$psol_binary" = "unset" ] ; then
+  if $build_from_source ; then
+    psol_binary="\
+        $mod_pagespeed_dir/pagespeed/automatic/pagespeed_automatic.a"
+  else
+    psol_library_dir="$ngx_addon_dir/psol/lib/$buildtype/$os_name/$arch_name"
+    psol_binary="$psol_library_dir/pagespeed_automatic.a"
+  fi
+fi
+
+echo "mod_pagespeed_dir=$mod_pagespeed_dir"
+echo "build_from_source=$build_from_source"
+
+ngx_feature="psol"
+ngx_feature_name=""
+ngx_feature_run=no
+ngx_feature_incs="
+#include \"pagespeed/kernel/base/string.h\"
+#include \"pagespeed/kernel/base/string_writer.h\"
+#include \"pagespeed/kernel/base/null_message_handler.h\"
+#include \"pagespeed/kernel/html/html_parse.h\"
+#include \"pagespeed/kernel/html/html_writer_filter.h\"
+"
+
 pagespeed_include="\
   $mod_pagespeed_dir \
   $mod_pagespeed_dir/third_party/chromium/src \
@@ -128,17 +162,10 @@ pagespeed_include="\
   $mod_pagespeed_dir/third_party/aprutil/gen/arch/$os_name/$arch_name/include"
 ngx_feature_path="$pagespeed_include"
 
-if $build_from_source ; then
-  psol_library_binaries="\
-    $mod_pagespeed_dir/net/instaweb/automatic/pagespeed_automatic.a"
-else
-  psol_library_dir="$ngx_addon_dir/psol/lib/$buildtype/$os_name/$arch_name"
-  psol_library_binaries="$psol_library_dir/pagespeed_automatic.a"
-fi
-
-pagespeed_libs="-lstdc++ $psol_library_binaries -lrt -pthread -lm"
+pagespeed_libs="$pagespeed_libs $psol_binary -lrt -pthread -lm"
 ngx_feature_libs="$pagespeed_libs"
 ngx_feature_test="
+
   GoogleString output_buffer;
   net_instaweb::StringWriter write_to_string(&output_buffer);
 
@@ -166,19 +193,24 @@ if [ $ngx_found = yes ]; then
     $ps_src/log_message_handler.h \
     $ps_src/ngx_base_fetch.h \
     $ps_src/ngx_caching_headers.h \
+    $ps_src/ngx_event_connection.h \
     $ps_src/ngx_fetch.h \
+    $ps_src/ngx_gzip_setter.h \
     $ps_src/ngx_list_iterator.h \
     $ps_src/ngx_message_handler.h \
     $ps_src/ngx_pagespeed.h \
     $ps_src/ngx_rewrite_driver_factory.h \
     $ps_src/ngx_rewrite_options.h \
     $ps_src/ngx_server_context.h \
-    $ps_src/ngx_url_async_fetcher.h"
+    $ps_src/ngx_url_async_fetcher.h \
+    $psol_binary"
   NGX_ADDON_SRCS="$NGX_ADDON_SRCS \
     $ps_src/log_message_handler.cc \
     $ps_src/ngx_base_fetch.cc \
     $ps_src/ngx_caching_headers.cc \
+    $ps_src/ngx_event_connection.cc \
     $ps_src/ngx_fetch.cc \
+    $ps_src/ngx_gzip_setter.cc \
     $ps_src/ngx_list_iterator.cc \
     $ps_src/ngx_message_handler.cc \
     $ps_src/ngx_pagespeed.cc \
@@ -186,13 +218,29 @@ if [ $ngx_found = yes ]; then
     $ps_src/ngx_rewrite_options.cc \
     $ps_src/ngx_server_context.cc \
     $ps_src/ngx_url_async_fetcher.cc"
+  # Save our sources in a separate var since we may need it in config.make
+  PS_NGX_SRCS="$NGX_ADDON_SRCS"
 
-  # Make pagespeed run immediately before gzip.
+  if [ "$position_aux" = "true" ] ; then
+     HTTP_AUX_FILTER_MODULES="$HTTP_AUX_FILTER_MODULES $ngx_addon_name"
+  else
+    # Make pagespeed run immediately before gzip and Brotli.
+    if echo $HTTP_FILTER_MODULES | grep ngx_http_brotli_filter_module >/dev/null; then
+      module=ngx_http_brotli_filter_module
+    elif [ $HTTP_GZIP = YES ]; then
+      module=$HTTP_GZIP_FILTER_MODULE
+    else
+      module=$HTTP_RANGE_HEADER_FILTER_MODULE
+    fi
+
+    HTTP_FILTER_MODULES=$(echo $HTTP_FILTER_MODULES |\
+      sed "s/$module/$module $ngx_addon_name/")
+  fi
+
+  # Make the etag header filter run immediately before range header filter.
   HTTP_FILTER_MODULES=$(echo $HTTP_FILTER_MODULES |\
-    sed "s/$HTTP_GZIP_FILTER_MODULE/$HTTP_GZIP_FILTER_MODULE $ngx_addon_name/")
-  # Make the etag header filter run immediately after gzip.
-  HTTP_FILTER_MODULES=$(echo $HTTP_FILTER_MODULES |\
-    sed "s/$HTTP_GZIP_FILTER_MODULE/ngx_pagespeed_etag_filter $HTTP_GZIP_FILTER_MODULE/")
+    sed "s/$HTTP_RANGE_HEADER_FILTER_MODULE/$HTTP_RANGE_HEADER_FILTER_MODULE ngx_pagespeed_etag_filter/")
+
   CORE_LIBS="$CORE_LIBS $pagespeed_libs"
   CORE_INCS="$CORE_INCS $pagespeed_include"
   echo "List of modules (in reverse order of applicability): "$HTTP_FILTER_MODULES
@@ -204,4 +252,46 @@ END
   exit 1
 fi
 
+# Test whether the compiler is compatible
+ngx_feature="psol-compiler-compat"
+ngx_feature_name=""
+ngx_feature_run=no
+ngx_feature_incs=""
+ngx_feature_path=""
+ngx_feature_libs="-lstdc++"
+ngx_feature_test="
+
+#if defined(__clang__) && defined(__GLIBCXX__)
+   // See https://gcc.gnu.org/onlinedocs/libstdc++/manual/abi.html#abi.versioning 
+   // for a list of various values of __GLIBCXX__. Note that they're not monotonic
+   // with respect to version numbers.
+   #if __GLIBCXX__ == 20120322  || __GLIBCXX__ == 20120614
+      #error \"clang is using libstdc++ 4.7.0 or 4.7.1, which can cause binary incompatibility.\"
+   #endif
+#endif
+
+#if !defined(__clang__) && defined(__GNUC__)
+   #if __GNUC__ < 4 || (__GNUC__ == 4 && __GNUC_MINOR__ < 8)
+      #error \"GCC < 4.8 no longer supported. Please use gcc >= 4.8 or clang >= 3.3\"
+   #endif
+#endif
+
+#if defined(__clang__)
+   #if __clang_major__ < 3 || (__clang_major__ == 3 && __clang_minor__ < 3)
+      #error \"Please use gcc >= 4.8 or clang >= 3.3\"
+   #endif
+#endif
+"
+
+. "$ngx_addon_dir/cpp_feature"
+
+if [ $ngx_found = no ]; then
+  cat << END
+$0: error: module ngx_pagespeed requires gcc >= 4.8 or clang >= 3.3.
+See https://developers.google.com/speed/pagespeed/module/build_ngx_pagespeed_from_source for some recommendations.
+Look in objs/autoconf.err for more details.
+END
+  exit 1
+fi
+
 have=NGX_PAGESPEED . auto/have

config.make

@@ -0,0 +1,7 @@
+if [ -n "$NGX_CLANG_VER" ]; then
+  # Chromium headers assume clang is always in C++11 mode. Oblige it.
+  for ps_src_file in $PS_NGX_SRCS; do
+    ps_obj_file="$NGX_OBJS/addon/src/`basename $ps_src_file .cc`.o"
+    echo "$ps_obj_file : CFLAGS += --std=c++11" >> $NGX_MAKEFILE
+  done
+fi

scripts/pagespeed_libraries_generator.sh

@@ -13,9 +13,9 @@
 # Author: vid@zippykid.com (Vid Luther)
 #         jefftk@google.com (Jeff Kaufman)
 
-URL="https://modpagespeed.googlecode.com/svn/trunk/src/"
+URL="https://github.com/pagespeed/mod_pagespeed/raw/master/"
 URL+="net/instaweb/genfiles/conf/pagespeed_libraries.conf"
-curl -s "$URL" \
+curl -L -s -S "$URL" \
     | grep ModPagespeedLibrary \
     | while read library size hash url ; do
   echo "  pagespeed Library $size $hash $url;"

src/log_message_handler.cc

@@ -29,7 +29,7 @@
 #include "base/debug/stack_trace.h"
 #include "base/logging.h"
 #include "net/instaweb/public/version.h"
-#include "net/instaweb/util/public/string_util.h"
+#include "pagespeed/kernel/base/string_util.h"
 
 // Make sure we don't attempt to use LOG macros here, since doing so
 // would cause us to go into an infinite log loop.
@@ -38,7 +38,7 @@
 
 namespace {
 
-ngx_log_t* log = NULL;
+ngx_log_t* ngx_log = NULL;
 
 ngx_uint_t GetNgxLogLevel(int severity) {
   switch (severity) {
@@ -78,7 +78,7 @@ bool LogMessageHandler(int severity, const char* file, int line,
     message.resize(last_msg_character_index);
   }
 
-  ngx_log_error(this_log_level, log, 0, "[ngx_pagespeed %s] %s",
+  ngx_log_error(this_log_level, ngx_log, 0, "[ngx_pagespeed %s] %s",
                 net_instaweb::kModPagespeedVersion,
                 message.c_str());
 
@@ -99,12 +99,12 @@ namespace log_message_handler {
 
 
 void Install(ngx_log_t* log_in) {
-  log = log_in;
+  ngx_log = log_in;
   logging::SetLogMessageHandler(&LogMessageHandler);
 
   // All VLOG(2) and higher will be displayed as DEBUG logs if the nginx log
   // level is DEBUG.
-  if (log->log_level >= NGX_LOG_DEBUG) {
+  if (ngx_log->log_level >= NGX_LOG_DEBUG) {
     logging::SetMinLogLevel(-2);
   }
 }

src/ngx_base_fetch.cc

@@ -15,37 +15,156 @@
  */
 
 // Author: jefftk@google.com (Jeff Kaufman)
+#include <unistd.h> //for usleep
 
 #include "ngx_base_fetch.h"
+#include "ngx_event_connection.h"
 #include "ngx_list_iterator.h"
 
 #include "ngx_pagespeed.h"
 
-#include "net/instaweb/http/public/response_headers.h"
 #include "net/instaweb/rewriter/public/rewrite_stats.h"
-#include "net/instaweb/util/public/google_message_handler.h"
-#include "net/instaweb/util/public/message_handler.h"
+#include "pagespeed/kernel/base/google_message_handler.h"
+#include "pagespeed/kernel/base/message_handler.h"
+#include "pagespeed/kernel/base/posix_timer.h"
+#include "pagespeed/kernel/http/response_headers.h"
 
 namespace net_instaweb {
 
-NgxBaseFetch::NgxBaseFetch(ngx_http_request_t* r, int pipe_fd,
+const char kHeadersComplete = 'H';
+const char kFlush = 'F';
+const char kDone = 'D';
+
+NgxEventConnection* NgxBaseFetch::event_connection = NULL;
+int NgxBaseFetch::active_base_fetches = 0;
+
+NgxBaseFetch::NgxBaseFetch(ngx_http_request_t* r,
                            NgxServerContext* server_context,
                            const RequestContextPtr& request_ctx,
-                           PreserveCachingHeaders preserve_caching_headers)
+                           PreserveCachingHeaders preserve_caching_headers,
+                           NgxBaseFetchType base_fetch_type)
     : AsyncFetch(request_ctx),
       request_(r),
       server_context_(server_context),
       done_called_(false),
       last_buf_sent_(false),
-      pipe_fd_(pipe_fd),
       references_(2),
-      handle_error_(true),
-      preserve_caching_headers_(preserve_caching_headers) {
+      base_fetch_type_(base_fetch_type),
+      preserve_caching_headers_(preserve_caching_headers),
+      detached_(false),
+      suppress_(false) {
   if (pthread_mutex_init(&mutex_, NULL)) CHECK(0);
+  __sync_add_and_fetch(&NgxBaseFetch::active_base_fetches, 1);
 }
 
 NgxBaseFetch::~NgxBaseFetch() {
   pthread_mutex_destroy(&mutex_);
+  __sync_add_and_fetch(&NgxBaseFetch::active_base_fetches, -1);
+}
+
+bool NgxBaseFetch::Initialize(ngx_cycle_t* cycle) {
+  CHECK(event_connection == NULL) << "event connection already set";
+  event_connection = new NgxEventConnection(ReadCallback);
+  return event_connection->Init(cycle);
+}
+
+void NgxBaseFetch::Terminate() {
+  if (event_connection != NULL) {
+    GoogleMessageHandler handler;
+    PosixTimer timer;
+    int64 timeout_us = Timer::kSecondUs * 30;
+    int64 end_us = timer.NowUs() + timeout_us;
+    static unsigned int sleep_microseconds = 100;
+
+    handler.Message(
+        kInfo,"NgxBaseFetch::Terminate rounding up %d active base fetches.",
+        NgxBaseFetch::active_base_fetches);
+
+    // Try to continue processing and get the active base fetch count to 0
+    // untill the timeout expires.
+    // TODO(oschaaf): This needs more work.
+    while (NgxBaseFetch::active_base_fetches > 0 && end_us > timer.NowUs()) {
+      event_connection->Drain();
+      usleep(sleep_microseconds);
+    }
+
+    if (NgxBaseFetch::active_base_fetches != 0) {
+      handler.Message(
+          kWarning,"NgxBaseFetch::Terminate timed out with %d active base fetches.",
+          NgxBaseFetch::active_base_fetches);
+    }
+
+    // Close down the named pipe.
+    event_connection->Shutdown();
+    delete event_connection;
+    event_connection = NULL;
+  }
+}
+
+const char* BaseFetchTypeToCStr(NgxBaseFetchType type) {
+  switch(type) {
+    case kPageSpeedResource:
+      return "ps resource";
+    case kHtmlTransform:
+      return "html transform";
+    case kAdminPage:
+      return "admin page";
+    case kIproLookup:
+      return "ipro lookup";
+    case kPageSpeedProxy:
+      return "pagespeed proxy";
+  }
+  CHECK(false);
+  return "can't get here";
+}
+
+void NgxBaseFetch::ReadCallback(const ps_event_data& data) {
+  NgxBaseFetch* base_fetch = reinterpret_cast<NgxBaseFetch*>(data.sender);
+  ngx_http_request_t* r = base_fetch->request();
+  bool detached = base_fetch->detached();
+#if (NGX_DEBUG)  // `type` is unused if NGX_DEBUG isn't set, needed for -Werror.
+  const char* type = BaseFetchTypeToCStr(base_fetch->base_fetch_type_);
+#endif
+  int refcount = base_fetch->DecrementRefCount();
+
+#if (NGX_DEBUG)
+  ngx_log_error(NGX_LOG_DEBUG, ngx_cycle->log, 0,
+     "pagespeed [%p] event: %c. bf:%p (%s) - refcnt:%d - det: %c", r,
+     data.type, base_fetch, type, refcount, detached ? 'Y': 'N');
+#endif
+
+  // If we ended up destructing the base fetch, or the request context is
+  // detached, skip this event.
+  if (refcount == 0 || detached) {
+    return;
+  }
+  ps_request_ctx_t* ctx = ps_get_request_context(r);
+
+  CHECK(data.sender == ctx->base_fetch);
+  CHECK(r->count > 0) << "r->count: " << r->count;
+
+  int rc;
+  // If we are unlucky enough to have our connection finalized mid-ipro-lookup,
+  // we must enter a different flow. Also see ps_in_place_check_header_filter().
+  if ((ctx->base_fetch->base_fetch_type_ != kIproLookup)
+      && r->connection->error) {
+    ngx_log_error(NGX_LOG_DEBUG, ngx_cycle->log, 0,
+      "pagespeed [%p] request already finalized", r);
+    rc = NGX_ERROR;
+  } else {
+    rc = ps_base_fetch::ps_base_fetch_handler(r);
+  }
+
+#if (NGX_DEBUG)
+  ngx_log_error(NGX_LOG_DEBUG, ngx_cycle->log, 0,
+                "pagespeed [%p] ps_base_fetch_handler() returned %d for %c",
+                r, rc, data.type);
+#endif
+
+  ngx_connection_t* c = r->connection;
+  ngx_http_finalize_request(r, rc);
+  // See http://forum.nginx.org/read.php?2,253006,253061
+  ngx_http_run_posted_requests(c);
 }
 
 void NgxBaseFetch::Lock() {
@@ -115,21 +234,19 @@ ngx_int_t NgxBaseFetch::CollectHeaders(ngx_http_headers_out_t* headers_out) {
                                       preserve_caching_headers_);
 }
 
-void NgxBaseFetch::RequestCollection() {
-  int rc;
-  char c = 'A';  // What byte we write is arbitrary.
-  while (true) {
-    rc = write(pipe_fd_, &c, 1);
-    if (rc == 1) {
-      break;
-    } else if (errno == EINTR || errno == EAGAIN || errno == EWOULDBLOCK) {
-      // TODO(jefftk): is this rare enough that spinning isn't a problem?  Could
-      // we get into a case where the pipe fills up and we spin forever?
+void NgxBaseFetch::RequestCollection(char type) {
+  if (suppress_) {
+    return;
+  }
 
-    } else {
-      perror("NgxBaseFetch::RequestCollection");
-      break;
-    }
+  // We must optimistically increment the refcount, and decrement it
+  // when we conclude we failed. If we only increment on a successfull write,
+  // there's a small chance that between writing and adding to the refcount
+  // both pagespeed and nginx will release their refcount -- destructing
+  // this NgxBaseFetch instance.
+  IncrementRefCount();
+  if (!event_connection->WriteEvent(type, this)) {
+    DecrementRefCount();
   }
 }
 
@@ -137,42 +254,54 @@ 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 ((base_fetch_type_ != kIproLookup) || 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.
+  RequestCollection(kHeadersComplete);  // Headers available.
+
+  // For the IPRO lookup, supress notification of the nginx side here.
+  // If we send both the headerscomplete 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.
+  if (base_fetch_type_ == kIproLookup && !status_ok) {
+    suppress_ = true;
+  }
 }
 
 bool NgxBaseFetch::HandleFlush(MessageHandler* handler) {
-  RequestCollection();  // A new part of the response body is available.
+  RequestCollection(kFlush);  // A new part of the response body is available
   return true;
 }
 
-void NgxBaseFetch::Release() {
-  DecrefAndDeleteIfUnreferenced();
+int NgxBaseFetch::DecrementRefCount() {
+  return DecrefAndDeleteIfUnreferenced();
 }
 
-void NgxBaseFetch::DecrefAndDeleteIfUnreferenced() {
+int NgxBaseFetch::IncrementRefCount() {
+  return __sync_add_and_fetch(&references_, 1);
+}
+
+int NgxBaseFetch::DecrefAndDeleteIfUnreferenced() {
   // Creates a full memory barrier.
-  if (__sync_add_and_fetch(&references_, -1) == 0) {
+  int r = __sync_add_and_fetch(&references_, -1);
+  if (r == 0) {
     delete this;
   }
+  return r;
 }
 
 void NgxBaseFetch::HandleDone(bool success) {
   // TODO(jefftk): it's possible that instead of locking here we can just modify
   // CopyBufferToNginx to only read done_called_ once.
+  CHECK(!done_called_) << "Done already called!";
   Lock();
   done_called_ = true;
   Unlock();
-
-  close(pipe_fd_);  // Indicates to nginx that we're done with the rewrite.
-  pipe_fd_ = -1;
-
+  RequestCollection(kDone);
   DecrefAndDeleteIfUnreferenced();
 }
 

src/ngx_base_fetch.h

@@ -17,22 +17,32 @@
 // Author: jefftk@google.com (Jeff Kaufman)
 //
 // Collects output from pagespeed and buffers it until nginx asks for it.
-// Notifies nginx via pipe to call CollectAccumulatedWrites() on flush.
+// Notifies nginx via NgxEventConnection to call ReadCallback() when
+// the headers are computed, when a flush should be performed, and when done.
 //
 //  - nginx creates a base fetch and passes it to a new proxy fetch.
 //  - The proxy fetch manages rewriting and thread complexity, and through
 //    several chained steps passes rewritten html to HandleWrite().
 //  - Written data is buffered.
-//  - When Flush() is called the base fetch writes a byte to a pipe nginx is
-//    watching so nginx knows to call CollectAccumulatedWrites() to pick up the
-//    rewritten html.
-//  - When Done() is called the base fetch closes the pipe, which tells nginx to
-//    make a final call to CollectAccumulatedWrites().
+//  - When HandleHeadersComplete(), HandleFlush(), or HandleDone() is called by
+//    PSOL, events are written to NgxEventConnection which will end up being
+//    handled by ReadCallback() on nginx's thread.
+//    When applicable, request processing will be continued via a call to
+//    ps_base_fetch_handler().
+//  - ps_base_fetch_handler() will pull the header and body bytes from PSOL
+//    via CollectAccumulatedWrites() and write those to the module's output.
 //
-// This class is referred two in two places: the proxy fetch and nginx's
-// request.  It must stay alive until both are finished.  The proxy fetch will
-// call Done() to indicate this; nginx will call Release().  Once both Done()
-// and Release() have been called this class will delete itself.
+// This class is referred to in three places: the proxy fetch, nginx's request,
+// and pending events written to the associated NgxEventConnection. It must stay
+// alive until the proxy fetch and nginx request are finished, and no more
+// events are pending.
+//  - The proxy fetch will call Done() to indicate this.
+//  - nginx will call Detach() when the associated request is handled
+//    completely (e.g. the request context is about to be destroyed).
+//  - ReadCallback() will call DecrementRefCount() on instances associated to
+//    events it handles.
+//
+// When the last reference is dropped, this class will delete itself.
 
 #ifndef NGX_BASE_FETCH_H_
 #define NGX_BASE_FETCH_H_
@@ -45,22 +55,43 @@ extern "C" {
 
 #include "ngx_pagespeed.h"
 
+#include "ngx_event_connection.h"
 #include "ngx_server_context.h"
 
 #include "net/instaweb/http/public/async_fetch.h"
-#include "net/instaweb/http/public/headers.h"
-#include "net/instaweb/util/public/string.h"
+#include "pagespeed/kernel/base/string.h"
+#include "pagespeed/kernel/http/headers.h"
 
 namespace net_instaweb {
 
+enum NgxBaseFetchType {
+  kIproLookup,
+  kHtmlTransform,
+  kPageSpeedResource,
+  kAdminPage,
+  kPageSpeedProxy
+};
+
 class NgxBaseFetch : public AsyncFetch {
  public:
-  NgxBaseFetch(ngx_http_request_t* r, int pipe_fd,
-               NgxServerContext* server_context,
+  NgxBaseFetch(ngx_http_request_t* r, NgxServerContext* server_context,
                const RequestContextPtr& request_ctx,
-               PreserveCachingHeaders preserve_caching_headers);
+               PreserveCachingHeaders preserve_caching_headers,
+               NgxBaseFetchType base_fetch_type);
   virtual ~NgxBaseFetch();
 
+  // Statically initializes event_connection, require for PSOL and nginx to
+  // communicate.
+  static bool Initialize(ngx_cycle_t* cycle);
+
+  // Attempts to finish up request processing queued up in the named pipe and
+  // PSOL for a fixed amount of time. If time is up, a fast and rough shutdown
+  // is attempted.
+  // Statically terminates and NULLS event_connection.
+  static void Terminate();
+
+  static void ReadCallback(const ps_event_data& data);
+
   // Puts a chain in link_ptr if we have any output data buffered.  Returns
   // NGX_OK on success, NGX_ERROR on errors.  If there's no data to send, sends
   // data only if Done() has been called.  Indicates the end of output by
@@ -77,9 +108,22 @@ class NgxBaseFetch : public AsyncFetch {
   // time for resource fetches.  Not called at all for proxy fetches.
   ngx_int_t CollectHeaders(ngx_http_headers_out_t* headers_out);
 
-  // Called by nginx when it's done with us.
-  void Release();
-  void set_handle_error(bool x) { handle_error_ = x; }
+  // Called by nginx to decrement the refcount.
+  int DecrementRefCount();
+
+  // Called by pagespeed to increment the refcount.
+  int IncrementRefCount();
+
+  // Detach() is called when the nginx side releases this base fetch. It
+  // sets detached_ to true and decrements the refcount. We need to know
+  // this to be able to handle events which nginx request context has been
+  // released while the event was in-flight.
+  void Detach() { detached_ = true; DecrementRefCount(); }
+
+  bool detached() { return detached_; }
+
+  ngx_http_request_t* request() { return request_; }
+  NgxBaseFetchType base_fetch_type() { return base_fetch_type_; }
 
  private:
   virtual bool HandleWrite(const StringPiece& sp, MessageHandler* handler);
@@ -89,7 +133,7 @@ class NgxBaseFetch : public AsyncFetch {
 
   // Indicate to nginx that we would like it to call
   // CollectAccumulatedWrites().
-  void RequestCollection();
+  void RequestCollection(char type);
 
   // Lock must be acquired first.
   // Returns:
@@ -105,20 +149,29 @@ class NgxBaseFetch : public AsyncFetch {
 
   // Called by Done() and Release().  Decrements our reference count, and if
   // it's zero we delete ourself.
-  void DecrefAndDeleteIfUnreferenced();
+  int DecrefAndDeleteIfUnreferenced();
+
+  static NgxEventConnection* event_connection;
+  
+  // Live count of NgxBaseFetch instances that are currently in use.
+  static int active_base_fetches;
 
   ngx_http_request_t* request_;
   GoogleString buffer_;
   NgxServerContext* server_context_;
   bool done_called_;
   bool last_buf_sent_;
-  int pipe_fd_;
   // How many active references there are to this fetch. Starts at two,
-  // decremented once when Done() is called and once when Release() is called.
+  // decremented once when Done() is called and once when Detach() is called.
+  // Incremented for each event written by pagespeed for this NgxBaseFetch, and
+  // decremented on the nginx side for each event read for it.
   int references_;
   pthread_mutex_t mutex_;
-  bool handle_error_;
+  NgxBaseFetchType base_fetch_type_;
   PreserveCachingHeaders preserve_caching_headers_;
+  // Set to true just before the nginx side releases its reference
+  bool detached_;
+  bool suppress_;
 
   DISALLOW_COPY_AND_ASSIGN(NgxBaseFetch);
 };

src/ngx_event_connection.cc

@@ -0,0 +1,172 @@
+/*
+ * Copyright 2014 Google Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+// Author: oschaaf@we-amp.com (Otto van der Schaaf)
+
+extern "C" {
+
+#include <ngx_channel.h>
+
+}
+
+#include "ngx_event_connection.h"
+
+#include "pagespeed/kernel/base/google_message_handler.h"
+#include "pagespeed/kernel/base/message_handler.h"
+
+namespace net_instaweb {
+
+  NgxEventConnection::NgxEventConnection(callbackPtr callback)
+    : event_handler_(callback) {
+}
+
+bool NgxEventConnection::Init(ngx_cycle_t* cycle) {
+  int file_descriptors[2];
+
+  if (pipe(file_descriptors) != 0) {
+    ngx_log_error(NGX_LOG_EMERG, cycle->log, 0, "pagespeed: pipe() failed");
+    return false;
+  }
+  if (ngx_nonblocking(file_descriptors[0]) == -1) {
+    ngx_log_error(NGX_LOG_EMERG, cycle->log, ngx_socket_errno,
+                  ngx_nonblocking_n "pagespeed:  pipe[0] failed");
+  } else if (ngx_nonblocking(file_descriptors[1]) == -1) {
+    ngx_log_error(NGX_LOG_EMERG, cycle->log, ngx_socket_errno,
+                  ngx_nonblocking_n "pagespeed:  pipe[1] failed");
+  } else if (!CreateNgxConnection(cycle, file_descriptors[0])) {
+    ngx_log_error(NGX_LOG_EMERG, cycle->log, 0,
+                  "pagespeed: failed to create connection.");
+  } else {
+    pipe_read_fd_ = file_descriptors[0];
+    pipe_write_fd_ = file_descriptors[1];
+    return true;
+  }
+  close(file_descriptors[0]);
+  close(file_descriptors[1]);
+  return false;
+}
+
+bool NgxEventConnection::CreateNgxConnection(ngx_cycle_t* cycle,
+                                             ngx_fd_t pipe_fd) {
+  // pipe_fd (the read side of the pipe will end up as c->fd on the
+  // underlying ngx_connection_t that gets created here)
+  ngx_int_t rc = ngx_add_channel_event(cycle, pipe_fd, NGX_READ_EVENT,
+      &NgxEventConnection::ReadEventHandler);
+  return rc  == NGX_OK;
+}
+
+void NgxEventConnection::ReadEventHandler(ngx_event_t* ev) {
+  ngx_connection_t* c = static_cast<ngx_connection_t*>(ev->data);
+  ngx_int_t result = ngx_handle_read_event(ev, 0);
+  if (result != NGX_OK) {
+    CHECK(false) << "pagespeed: ngx_handle_read_event error: " << result;
+  }
+
+  if (ev->timedout) {
+    ev->timedout = 0;
+    return;
+  }
+
+  if (!NgxEventConnection::ReadAndNotify(c->fd)) {
+    // This was copied from ngx_channel_handler(): for epoll, we need to call
+    // ngx_del_conn(). Sadly, no documentation as to why.
+    if (ngx_event_flags & NGX_USE_EPOLL_EVENT) {
+      ngx_del_conn(c, 0);
+    }
+    ngx_close_connection(c);
+    ngx_del_event(ev, NGX_READ_EVENT, 0);
+  }
+}
+
+// Deserialize ps_event_data's from the pipe as they become available.
+// Subsequently do some bookkeeping, cleanup, and error checking to keep
+// the mess out of ps_base_fetch_handler.
+bool NgxEventConnection::ReadAndNotify(ngx_fd_t fd) {
+  while (true) {
+    // We read only one ps_event_data at a time for now:
+    // We can end up recursing all the way and end up calling ourselves here.
+    // If that happens in the middle of looping over multiple ps_event_data's we
+    // have obtained with read(), the results from the next read() will make us
+    // process events out of order. Which can give headaches.
+    // Alternatively, we could maintain a queue to make sure we process in
+    // sequence
+    ps_event_data data;
+    ngx_int_t size = read(fd, static_cast<void*>(&data), sizeof(data));
+
+    if (size == -1) {
+      if (errno == EINTR) {
+        continue;
+      // TODO(oschaaf): should we worry about spinning here?
+      } else if (ngx_errno == EAGAIN || ngx_errno == EWOULDBLOCK) {
+        return true;
+      }
+    }
+
+    if (size <= 0) {
+      return false;
+    }
+
+    data.connection->event_handler_(data);
+    return true;
+  }
+}
+
+bool NgxEventConnection::WriteEvent(void* sender) {
+  return WriteEvent('X' /* Anything char is fine */, sender);
+}
+
+bool NgxEventConnection::WriteEvent(char type, void* sender) {
+  ssize_t size = 0;
+  ps_event_data data;
+
+  ngx_memzero(&data, sizeof(data));
+  data.type = type;
+  data.sender = sender;
+  data.connection = this;
+
+  while (true) {
+    size = write(pipe_write_fd_,
+                 static_cast<void*>(&data), sizeof(data));
+    if (size == sizeof(data)) {
+      return true;
+    } else if (size == -1) {
+      // TODO(oschaaf): should we worry about spinning here?
+      if (ngx_errno == EINTR || ngx_errno == EAGAIN
+          || ngx_errno == EWOULDBLOCK) {
+        continue;
+      } else {
+        return false;
+      }
+    } else {
+      CHECK(false) << "pagespeed: unexpected return value from write(): "
+                   << size;
+    }
+  }
+  CHECK(false) << "Should not get here";
+  return false;
+}
+
+// Reads and processes what is available in the pipe.
+void NgxEventConnection::Drain() {
+  NgxEventConnection::ReadAndNotify(pipe_read_fd_);
+}
+
+void NgxEventConnection::Shutdown() {
+  close(pipe_write_fd_);
+  close(pipe_read_fd_);
+}
+
+}  // namespace net_instaweb

src/ngx_event_connection.h

@@ -0,0 +1,84 @@
+/*
+ * Copyright 2014 Google Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+// Author: oschaaf@we-amp.com (Otto van der Schaaf)
+//
+// NgxEventConnection implements a means to send events from other threads to
+// nginx's event loop, and is implemented by a named pipe under the hood.
+// A single instance is used by NgxBaseFetch, and one instance is created per
+// NgxUrlAsyncFetcher when native fetching is on.
+
+#ifndef NGX_EVENT_CONNECTION_H_
+#define NGX_EVENT_CONNECTION_H_
+
+extern "C" {
+#include <ngx_http.h>
+}
+
+#include <pthread.h>
+
+#include "pagespeed/kernel/base/string.h"
+#include "pagespeed/kernel/http/headers.h"
+
+namespace net_instaweb {
+
+class NgxEventConnection;
+
+// Represents a single event that can be written to or read from the pipe.
+// Technically, sender is the only data we need to send. type and connection are
+// included to provide a means to trace the events along with some more
+// info.
+typedef struct {
+  char type;
+  void* sender;
+  NgxEventConnection* connection;
+} ps_event_data;
+
+// Handler signature for receiving events
+typedef void (*callbackPtr)(const ps_event_data&);
+
+// Abstracts a connection to nginx through which events can be written.
+class NgxEventConnection {
+ public:
+  explicit NgxEventConnection(callbackPtr handler);
+
+  // Creates the file descriptors and ngx_connection_t required for event
+  // messaging between pagespeed and nginx.
+  bool Init(ngx_cycle_t* cycle);
+  // Shuts down the underlying file descriptors and connection created in Init()
+  void Shutdown();
+  // Constructs a ps_event_data and writes it to the underlying named pipe.
+  bool WriteEvent(char type, void* sender);
+  // Convenience overload for clients that have a single event type.
+  bool WriteEvent(void* sender);
+  // Reads and processes what is available in the named pipe's buffer.
+  void Drain();
+ private:
+  static bool CreateNgxConnection(ngx_cycle_t* cycle, ngx_fd_t pipe_fd);
+  static void ReadEventHandler(ngx_event_t* e);
+  static bool ReadAndNotify(ngx_fd_t fd);
+
+  callbackPtr event_handler_;
+  // We own these file descriptors
+  ngx_fd_t pipe_write_fd_;
+  ngx_fd_t pipe_read_fd_;
+
+  DISALLOW_COPY_AND_ASSIGN(NgxEventConnection);
+};
+
+}  // namespace net_instaweb
+
+#endif  // NGX_EVENT_CONNECTION_H_

src/ngx_fetch.h

@@ -16,6 +16,15 @@
 
 // Author: x.dinic@gmail.com (Junmin Xiong)
 //
+// PageSpeed needs some way to talk to the internet and request resources.  For
+// example, if it's optimizing www.example.com/index.html and it sees html with
+// <img src="//images.example.com/cat.jpg"> and images.example.com is authorized
+// for rewriting in the config, then it needs to fetch cat.jpg from
+// images.example.com and optimize it.  In apache (always) and nginx (by
+// default) we use a fetcher called "serf".  This works fine, but it does run
+// its own event loop.  To be more efficient, this is a "native" fetcher that
+// uses nginx's event loop.
+//
 // The fetch is started by the main thread. It will fetch the remote resource
 // from the specific url asynchronously.
 
@@ -30,24 +39,72 @@ extern "C" {
 
 #include "ngx_url_async_fetcher.h"
 #include <vector>
-#include "net/instaweb/util/public/basictypes.h"
-#include "net/instaweb/util/public/pool.h"
-#include "net/instaweb/util/public/string.h"
 #include "net/instaweb/http/public/url_async_fetcher.h"
-#include "net/instaweb/http/public/response_headers.h"
-#include "net/instaweb/http/public/response_headers_parser.h"
+#include "pagespeed/kernel/base/basictypes.h"
+#include "pagespeed/kernel/base/pool.h"
+#include "pagespeed/kernel/base/string.h"
+#include "pagespeed/kernel/http/response_headers.h"
+#include "pagespeed/kernel/http/response_headers_parser.h"
+#include "pagespeed/kernel/thread/pthread_mutex.h"
+
 
 namespace net_instaweb {
 
 typedef bool (*response_handler_pt)(ngx_connection_t* c);
 
 class NgxUrlAsyncFetcher;
+class NgxConnection;
+
+class NgxConnection : public PoolElement<NgxConnection> {
+ public:
+  NgxConnection(MessageHandler* handler, int max_keepalive_requests);
+  ~NgxConnection();
+  void SetSock(u_char *sockaddr, socklen_t socklen) {
+    socklen_ = socklen;
+    ngx_memcpy(&sockaddr_, sockaddr, socklen);
+  }
+  // Close ensures that NgxConnection deletes itself at the appropriate time,
+  // which can be after receiving a non-keepalive response, or when the remote
+  // server closes the connection when the NgxConnection is pooled and idle.
+  void Close();
+
+  // Once keepalive is disabled, it can't be toggled back on.
+  void set_keepalive(bool k) { keepalive_ = keepalive_ && k; }
+  bool keepalive() { return keepalive_; }
+
+  typedef Pool<NgxConnection> NgxConnectionPool;
+
+  static NgxConnection* Connect(ngx_peer_connection_t* pc,
+                                MessageHandler* handler,
+                                int max_keepalive_requests);
+  static void IdleWriteHandler(ngx_event_t* ev);
+  static void IdleReadHandler(ngx_event_t* ev);
+  // Terminate will cleanup any idle connections upon shutdown.
+  static void Terminate();
+
+  static NgxConnectionPool connection_pool;
+  static PthreadMutex connection_pool_mutex;
+
+  // c_ is owned by NgxConnection and freed in ::Close()
+  ngx_connection_t* c_;
+  static const int64 keepalive_timeout_ms;
+  static const GoogleString ka_header;
+
+ private:
+  int max_keepalive_requests_;
+  bool keepalive_;
+  socklen_t socklen_;
+  u_char sockaddr_[NGX_SOCKADDRLEN];
+  MessageHandler* handler_;
+
+  DISALLOW_COPY_AND_ASSIGN(NgxConnection);
+};
+
 class NgxFetch : public PoolElement<NgxFetch> {
  public:
   NgxFetch(const GoogleString& url,
            AsyncFetch* async_fetch,
            MessageHandler* message_handler,
-           ngx_msec_t timeout_ms,
            ngx_log_t* log);
   ~NgxFetch();
 
@@ -103,19 +160,19 @@ class NgxFetch : public PoolElement<NgxFetch> {
     response_handler = handler;
   }
   // Only the Static functions could be used in callbacks.
-  static void NgxFetchResolveDone(ngx_resolver_ctx_t* ctx);
+  static void ResolveDoneHandler(ngx_resolver_ctx_t* ctx);
   // Write the request.
-  static void NgxFetchWrite(ngx_event_t* wev);
+  static void ConnectionWriteHandler(ngx_event_t* wev);
   // Wait for the response.
-  static void NgxFetchRead(ngx_event_t* rev);
+  static void ConnectionReadHandler(ngx_event_t* rev);
   // Read and parse the first status line.
-  static bool NgxFetchHandleStatusLine(ngx_connection_t* c);
+  static bool HandleStatusLine(ngx_connection_t* c);
   // Read and parse the HTTP headers.
-  static bool NgxFetchHandleHeader(ngx_connection_t* c);
+  static bool HandleHeader(ngx_connection_t* c);
   // Read the response body.
-  static bool NgxFetchHandleBody(ngx_connection_t* c);
+  static bool HandleBody(ngx_connection_t* c);
   // Cancel the fetch when it's timeout.
-  static void NgxFetchTimeout(ngx_event_t* tev);
+  static void TimeoutHandler(ngx_event_t* tev);
 
   // Add the pagespeed User-Agent.
   void FixUserAgent();
@@ -130,7 +187,6 @@ class NgxFetch : public PoolElement<NgxFetch> {
   int64 bytes_received_;
   int64 fetch_start_ms_;
   int64 fetch_end_ms_;
-  int64 timeout_ms_;
   bool done_;
   int64 content_length_;
   bool content_length_known_;
@@ -143,7 +199,7 @@ class NgxFetch : public PoolElement<NgxFetch> {
   ngx_http_request_t* r_;
   ngx_http_status_t* status_;
   ngx_event_t* timeout_event_;
-  ngx_connection_t* connection_;
+  NgxConnection* connection_;
   ngx_resolver_ctx_t* resolver_ctx_;
 
   DISALLOW_COPY_AND_ASSIGN(NgxFetch);

src/ngx_gzip_setter.cc

@@ -0,0 +1,403 @@
+/*
+ * Copyright 2014 Google Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+// Author: kspoelstra@we-amp.com (Kees Spoelstra)
+
+#include "ngx_gzip_setter.h"
+
+#include <ngx_conf_file.h>
+
+namespace net_instaweb {
+
+NgxGZipSetter g_gzip_setter;
+
+extern "C" {
+  // These functions replace the setters for:
+  //   gzip
+  //   gzip_types
+  //   gzip_http_version
+  //   gzip_vary
+  //
+  // If these functions are called it means there is an explicit gzip
+  // configuration. The gzip configuration set by pagespeed is then rolled
+  // back and pagespeed will stop enabling gzip automatically.
+  char* ngx_gzip_redirect_conf_set_flag_slot(
+      ngx_conf_t* cf, ngx_command_t* cmd, void* conf) {
+    if (g_gzip_setter.enabled()) {
+      g_gzip_setter.RollBackAndDisable(cf);
+    }
+    char* ret = ngx_conf_set_flag_slot(cf, cmd, conf);
+    return ret;
+  }
+
+  char* ngx_gzip_redirect_http_types_slot(
+      ngx_conf_t* cf, ngx_command_t* cmd, void* conf) {
+    if (g_gzip_setter.enabled()) {
+      g_gzip_setter.RollBackAndDisable(cf);
+    }
+    char* ret = ngx_http_types_slot(cf, cmd, conf);
+    return ret;
+  }
+
+  char* ngx_gzip_redirect_conf_set_enum_slot(
+      ngx_conf_t* cf, ngx_command_t* cmd, void* conf) {
+    if (g_gzip_setter.enabled()) {
+      g_gzip_setter.RollBackAndDisable(cf);
+    }
+    char* ret = ngx_conf_set_enum_slot(cf, cmd, conf);
+    return ret;
+  }
+  char* ngx_gzip_redirect_conf_set_bitmask_slot(
+      ngx_conf_t* cf, ngx_command_t* cmd, void* conf) {
+    if (g_gzip_setter.enabled()) {
+      g_gzip_setter.RollBackAndDisable(cf);
+    }
+    char* ret = ngx_conf_set_bitmask_slot(cf, cmd, conf);
+    return ret;
+  }
+}
+
+NgxGZipSetter::NgxGZipSetter() : enabled_(0) { }
+NgxGZipSetter::~NgxGZipSetter() { }
+
+// Helper functions to determine signature.
+bool HasLocalConfig(ngx_command_t* command) {
+  return (!(command->type & (NGX_DIRECT_CONF|NGX_MAIN_CONF)) &&
+          command->conf == NGX_HTTP_LOC_CONF_OFFSET);
+}
+bool IsNgxFlagCommand(ngx_command_t* command) {
+  return (command->set == ngx_conf_set_flag_slot &&
+          HasLocalConfig(command));
+}
+bool IsNgxHttpTypesCommand(ngx_command_t* command) {
+  return (command->set == ngx_http_types_slot &&
+          HasLocalConfig(command));
+}
+bool IsNgxEnumCommand(ngx_command_t* command) {
+  return (command->set == ngx_conf_set_enum_slot &&
+          HasLocalConfig(command));
+}
+bool IsNgxBitmaskCommand(ngx_command_t* command) {
+  return (command->set == ngx_conf_set_bitmask_slot &&
+          HasLocalConfig(command));
+}
+
+// Initialize the NgxGzipSetter.
+// Find the gzip, gzip_vary, gzip_http_version and gzip_types commands in the
+// gzip module. Enable if the signature of the zip command matches with what we
+// trust. Also sets up redirects for the configurations. These redirect handle
+// a rollback if expicit configuration is found.
+// If commands are not found the method will inform the user by logging.
+void NgxGZipSetter::Init(ngx_conf_t* cf) {
+#if (NGX_HTTP_GZIP)
+  bool gzip_signature_mismatch = false;
+  bool other_signature_mismatch = false;
+  for (int m = 0; ngx_modules[m] != NULL; m++) {
+    if (ngx_modules[m]->commands != NULL) {
+      for (int c = 0; ngx_modules[m]->commands[c].name.len; c++) {
+        ngx_command_t* current_command =& ngx_modules[m]->commands[c];
+
+        // We look for the gzip command, and the exact signature we trust
+        // this means configured as an config location offset
+        // and a ngx_flag_t setter.
+        // Also see:
+        //   ngx_conf_handler in ngx_conf_file.c
+        //   ngx_http_gzip_filter_commands in ngx_http_gzip_filter.c
+        if (gzip_command_.command_ == NULL &&
+            STR_EQ_LITERAL(current_command->name, "gzip")) {
+          if (IsNgxFlagCommand(current_command)) {
+            current_command->set = ngx_gzip_redirect_conf_set_flag_slot;
+            gzip_command_.command_ = current_command;
+            gzip_command_.module_ = ngx_modules[m];
+            enabled_ = 1;
+          } else {
+            ngx_conf_log_error(
+                NGX_LOG_WARN, cf, 0,
+                "pagespeed: cannot set gzip, signature mismatch");
+            gzip_signature_mismatch = true;
+          }
+        }
+
+        if (!gzip_http_version_command_.command_ &&
+            STR_EQ_LITERAL(current_command->name, "gzip_http_version")) {
+          if (IsNgxEnumCommand(current_command)) {
+            current_command->set = ngx_gzip_redirect_conf_set_enum_slot;
+            gzip_http_version_command_.command_ = current_command;
+            gzip_http_version_command_.module_ = ngx_modules[m];
+          } else {
+            ngx_conf_log_error(
+                NGX_LOG_WARN, cf, 0,
+                "pagespeed: cannot set gzip_http_version, signature mismatch");
+            other_signature_mismatch = true;
+          }
+        }
+
+        if (!gzip_proxied_command_.command_ &&
+            STR_EQ_LITERAL(current_command->name, "gzip_proxied")) {
+          if (IsNgxBitmaskCommand(current_command)) {
+            current_command->set = ngx_gzip_redirect_conf_set_bitmask_slot;
+            gzip_proxied_command_.command_ = current_command;
+            gzip_proxied_command_.module_ = ngx_modules[m];
+          } else {
+            ngx_conf_log_error(
+                NGX_LOG_WARN, cf, 0,
+                "pagespeed: cannot set gzip_proxied, signature mismatch");
+            other_signature_mismatch = true;
+          }
+        }
+
+        if (!gzip_http_types_command_.command_ &&
+            STR_EQ_LITERAL(current_command->name, "gzip_types")) {
+          if (IsNgxHttpTypesCommand(current_command)) {
+            current_command->set = ngx_gzip_redirect_http_types_slot;
+            gzip_http_types_command_.command_ = current_command;
+            gzip_http_types_command_.module_ = ngx_modules[m];
+          } else {
+            ngx_conf_log_error(
+                NGX_LOG_WARN, cf, 0,
+                "pagespeed: cannot set gzip_types, signature mismatch");
+            other_signature_mismatch = true;
+          }
+        }
+
+        if (!gzip_vary_command_.command_ &&
+            STR_EQ_LITERAL(current_command->name, "gzip_vary")) {
+          if (IsNgxFlagCommand(current_command)) {
+            current_command->set = ngx_gzip_redirect_conf_set_flag_slot;
+            gzip_vary_command_.command_ = current_command;
+            gzip_vary_command_.module_ = ngx_modules[m];
+          } else {
+            ngx_conf_log_error(
+                NGX_LOG_WARN, cf, 0,
+                "pagespeed: cannot set gzip_vary, signature mismatch");
+            other_signature_mismatch = true;
+          }
+        }
+      }
+    }
+  }
+  if (gzip_signature_mismatch) {
+    return;  // Already logged error.
+  } else if (!enabled_) {
+    // Looked through all the available commands and didn't find the "gzip" one.
+    ngx_conf_log_error(
+        NGX_LOG_WARN, cf, 0, "pagespeed: cannot set gzip, command not found");
+    return;
+  } else if (other_signature_mismatch) {
+    return;  // Already logged error.
+  } else if (!gzip_vary_command_.command_) {
+    ngx_conf_log_error(
+        NGX_LOG_WARN, cf, 0, "pagespeed: missing gzip_vary");
+    return;
+  } else if (!gzip_http_types_command_.command_) {
+    ngx_conf_log_error(
+        NGX_LOG_WARN, cf, 0, "pagespeed: missing gzip_types");
+    return;
+  } else if (!gzip_http_version_command_.command_) {
+    ngx_conf_log_error(
+        NGX_LOG_WARN, cf, 0, "pagespeed: missing gzip_http_version");
+    return;
+  } else if (!gzip_proxied_command_.command_) {
+    ngx_conf_log_error(
+        NGX_LOG_WARN, cf, 0, "pagespeed: missing gzip_proxied");
+    return;
+  } else {
+    return;  // Success.
+  }
+#else
+  ngx_conf_log_error(
+      NGX_LOG_WARN, cf, 0, "pagespeed: gzip not compiled into nginx");
+  return;
+#endif
+}
+
+void* ngx_command_ctx::GetConfPtr(ngx_conf_t* cf) {
+  return GetModuleConfPtr(cf) + command_->offset;
+}
+
+char* ngx_command_ctx::GetModuleConfPtr(ngx_conf_t* cf) {
+  return reinterpret_cast<char*>(
+      ngx_http_conf_get_module_loc_conf(cf, (*(module_))));
+}
+
+void NgxGZipSetter::SetNgxConfFlag(ngx_conf_t* cf,
+                                   ngx_command_ctx* command_ctx,
+                                   ngx_flag_t value) {
+  ngx_flag_t* flag = reinterpret_cast<ngx_flag_t*>(command_ctx->GetConfPtr(cf));
+  *flag = value;
+  // Save the flag position for possible rollback.
+  ngx_flags_set_.push_back(flag);
+}
+
+void NgxGZipSetter::SetNgxConfEnum(ngx_conf_t* cf,
+                                   ngx_command_ctx* command_ctx,
+                                   ngx_uint_t value) {
+  ngx_uint_t* enum_to_set =
+      reinterpret_cast<ngx_uint_t*>(command_ctx->GetConfPtr(cf));
+  *enum_to_set = value;
+  ngx_uint_set_.push_back(enum_to_set);
+}
+
+void NgxGZipSetter::SetNgxConfBitmask(ngx_conf_t* cf,
+                                      ngx_command_ctx* command_ctx,
+                                      ngx_uint_t value) {
+  ngx_uint_t* enum_to_set =
+      reinterpret_cast<ngx_uint_t*>(command_ctx->GetConfPtr(cf));
+  *enum_to_set = value;
+  ngx_uint_set_.push_back(enum_to_set);
+}
+
+// These are the content types we want to compress.
+ngx_str_t gzip_http_types[] = {
+  ngx_string("application/ecmascript"),
+  ngx_string("application/javascript"),
+  ngx_string("application/json"),
+  ngx_string("application/pdf"),
+  ngx_string("application/postscript"),
+  ngx_string("application/x-javascript"),
+  ngx_string("image/svg+xml"),
+  ngx_string("text/css"),
+  ngx_string("text/csv"),
+  // ngx_string("text/html"),  // This is the default implied value.
+  ngx_string("text/javascript"),
+  ngx_string("text/plain"),
+  ngx_string("text/xml"),
+  ngx_null_string  // Indicates end of array.
+};
+
+gzs_enable_result NgxGZipSetter::SetGZipForLocation(ngx_conf_t* cf,
+                                                    bool value) {
+  if (!enabled_) {
+    return kEnableGZipNotEnabled;
+  }
+  if (gzip_command_.command_) {
+    SetNgxConfFlag(cf, &gzip_command_, value);
+  }
+  return kEnableGZipOk;
+}
+
+void NgxGZipSetter::EnableGZipForLocation(ngx_conf_t* cf) {
+  if (!enabled_) {
+    return;
+  }
+
+  // When we get called twice for the same location{}, we ignore the second call
+  // to prevent adding duplicate gzip http types and so on.
+  ngx_flag_t* flag =
+      reinterpret_cast<ngx_flag_t*>(gzip_command_.GetConfPtr(cf));
+  if (*flag == 1) {
+    return;
+  }
+  SetGZipForLocation(cf, true);
+  if (gzip_vary_command_.command_) {
+    SetNgxConfFlag(cf, &gzip_vary_command_, 1);
+  }
+  if (gzip_http_version_command_.command_) {
+    SetNgxConfEnum(cf, &gzip_http_version_command_, NGX_HTTP_VERSION_10);
+  }
+  if (gzip_proxied_command_.command_) {
+    SetNgxConfBitmask(
+        cf, &gzip_http_version_command_, NGX_HTTP_GZIP_PROXIED_ANY);
+  }
+
+  // This is actually the most prone to future API changes, because gzip_types
+  // is not a simple type like ngx_flag_t. The signature check should be enough
+  // to prevent problems.
+  AddGZipHTTPTypes(cf);
+  return;
+}
+
+void NgxGZipSetter::AddGZipHTTPTypes(ngx_conf_t* cf) {
+  if (gzip_http_types_command_.command_) {
+    // Following should not happen, but if it does return gracefully.
+    if (cf->args->nalloc < 2) {
+      ngx_conf_log_error(NGX_LOG_WARN, cf, 0,
+                         "pagespeed: unexpected small cf->args in gzip_types");
+      return;
+    }
+
+    ngx_command_t* command = gzip_http_types_command_.command_;
+    char* gzip_conf = reinterpret_cast<char* >(
+        gzip_http_types_command_.GetModuleConfPtr(cf));
+
+    // Backup the old settings.
+    ngx_str_t old_elt0 = reinterpret_cast<ngx_str_t*>(cf->args->elts)[0];
+    ngx_str_t old_elt1 = reinterpret_cast<ngx_str_t*>(cf->args->elts)[1];
+    ngx_uint_t old_nelts = cf->args->nelts;
+
+    // Setup first arg.
+    ngx_str_t gzip_types_string = ngx_string("gzip_types");
+    reinterpret_cast<ngx_str_t*>(cf->args->elts)[0] = gzip_types_string;
+    cf->args->nelts = 2;
+
+    ngx_str_t* http_types = gzip_http_types;
+    while (http_types->data) {
+      ngx_str_t d;
+      // We allocate the http type on the configuration pool and actually
+      // leak this if we rollback. This does not seem to be a big problem,
+      // because nginx also allocates tokens in ngx_conf_file.c and does not
+      // free them. This way they can be used safely by configurations.
+      // We must use a copy of gzip_http_types array here because nginx will
+      // manipulate the values.
+      // TODO(kspoelstra): better would be to allocate once on init and not
+      // every time we enable gzip. This needs further investigation, sharing
+      // tokens might be problematic.
+      // For now I think it is not a large problem. This might add up in case
+      // of a large multi server/location config with a lot of "pagespeed on"
+      // directives.
+      // Estimates are 300-400KB for 1000 times "pagespeed on".
+      d.data = reinterpret_cast<u_char*>(
+          ngx_pnalloc(cf->pool, http_types->len + 1));
+      snprintf(reinterpret_cast<char*>(d.data), http_types->len + 1, "%s",
+               reinterpret_cast<const char*>(http_types->data));
+      d.len = http_types->len;
+      reinterpret_cast<ngx_str_t*>(cf->args->elts)[1] = d;
+      // Call the original setter.
+      ngx_http_types_slot(cf, command, gzip_conf);
+      http_types++;
+    }
+
+    // Restore args.
+    cf->args->nelts = old_nelts;
+    reinterpret_cast<ngx_str_t*>(cf->args->elts)[1] = old_elt1;
+    reinterpret_cast<ngx_str_t*>(cf->args->elts)[0] = old_elt0;
+
+    // Backup configuration location for rollback.
+    ngx_httptypes_set_.push_back(gzip_conf + command->offset);
+  }
+}
+
+void NgxGZipSetter::RollBackAndDisable(ngx_conf_t* cf) {
+  ngx_conf_log_error(NGX_LOG_INFO, cf, 0,
+                     "pagespeed: rollback gzip, explicit configuration");
+  for (std::vector<ngx_flag_t*>::iterator i = ngx_flags_set_.begin();
+       i != ngx_flags_set_.end(); ++i) {
+    *(*i)=NGX_CONF_UNSET;
+  }
+  for (std::vector<ngx_uint_t*>::iterator i = ngx_uint_set_.begin();
+       i != ngx_uint_set_.end(); ++i) {
+    *(*i)=NGX_CONF_UNSET_UINT;
+  }
+  for (std::vector<void*>::iterator i = ngx_httptypes_set_.begin();
+       i != ngx_httptypes_set_.end(); ++i) {
+    ngx_array_t** type_array = reinterpret_cast<ngx_array_t**>(*i);
+    ngx_array_destroy(*type_array);
+    *type_array = NULL;
+  }
+  enabled_ = 0;
+}
+
+}  // namespace net_instaweb

src/ngx_gzip_setter.h

@@ -0,0 +1,124 @@
+/*
+ * Copyright 2014 Google Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+// Author: kspoelstra@we-amp.com (Kees Spoelstra)
+
+/*
+ * NgxGZipSetter sets up gzip for pagespeed
+ * with the following configuration:
+ * gzip  on;
+ * gzip_vary on;
+ * gzip_types application/ecmascript;
+ * gzip_types application/javascript;
+ * gzip_types application/json;
+ * gzip_types application/pdf;
+ * gzip_types application/postscript;
+ * gzip_types application/x-javascript;
+ * gzip_types image/svg+xml;
+ * gzip_types text/css;
+ * gzip_types text/csv;
+ * gzip_types text/javascript;
+ * gzip_types text/plain;
+ * gzip_types text/xml;
+ * gzip_http_version 1.0;
+ *
+ * If there is an explicit gzip configuration in the nginx.conf
+ * pagespeed will rollback the set configuration and let the
+ * user decide what the configuration will be.
+ *
+ * It manipulates the configuration by manipulating ngx_flag_t
+ * and ngx_uint_t settings directly and using the nginx setter for
+ * gzip_http_types.
+ * This is probably a safe way to do it. If this mechanism
+ * changes all non nginx module setup & configuration will
+ * fail.
+ */
+
+#ifndef NGX_GZIP_SETTER_H_
+#define NGX_GZIP_SETTER_H_
+
+extern "C" {
+#include <ngx_config.h>
+#include <ngx_core.h>
+#include <ngx_http.h>
+}
+
+#include <vector>
+
+#include "ngx_pagespeed.h"
+
+#include "pagespeed/kernel/base/basictypes.h"
+
+namespace net_instaweb {
+
+// We need this class because configuration for gzip is in different modules, so
+// just saving the command will not work.
+class ngx_command_ctx {
+ public:
+  ngx_command_ctx() : command_(NULL), module_(NULL) { }
+  void* GetConfPtr(ngx_conf_t* cf);
+  char* GetModuleConfPtr(ngx_conf_t* cf);
+  ngx_command_t* command_;
+  ngx_module_t* module_;
+};
+
+enum gzs_enable_result {
+  kEnableGZipOk,
+  kEnableGZipPartial,
+  kEnableGZipNotEnabled
+};
+
+class NgxGZipSetter {
+  std::vector<ngx_flag_t*> ngx_flags_set_;
+  std::vector<ngx_uint_t*> ngx_uint_set_;
+  std::vector<void*> ngx_httptypes_set_;
+  ngx_command_ctx gzip_command_;
+  ngx_command_ctx gzip_http_types_command_;
+  ngx_command_ctx gzip_proxied_command_;
+  ngx_command_ctx gzip_vary_command_;
+  ngx_command_ctx gzip_http_version_command_;
+  bool enabled_;
+
+ public:
+  NgxGZipSetter();
+  ~NgxGZipSetter();
+  void Init(ngx_conf_t* cf);
+
+  void SetNgxConfFlag(ngx_conf_t* cf,
+                      ngx_command_ctx* command_ctx,
+                      ngx_flag_t value);
+  void SetNgxConfEnum(ngx_conf_t* cf,
+                      ngx_command_ctx* command_ctx,
+                      ngx_uint_t value);
+  void SetNgxConfBitmask(ngx_conf_t* cf,
+                         ngx_command_ctx* command_ctx,
+                         ngx_uint_t value);
+  void EnableGZipForLocation(ngx_conf_t* cf);
+  gzs_enable_result SetGZipForLocation(ngx_conf_t* cf, bool value);
+  void AddGZipHTTPTypes(ngx_conf_t* cf);
+  void RollBackAndDisable(ngx_conf_t* cf);
+
+  bool enabled() { return enabled_; }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(NgxGZipSetter);
+};
+
+extern NgxGZipSetter g_gzip_setter;
+
+}  // namespace net_instaweb
+
+#endif  // NGX_GZIP_SETTER_H_

src/ngx_message_handler.cc

@@ -18,13 +18,13 @@
 
 #include <signal.h>
 
-#include "net/instaweb/util/public/abstract_mutex.h"
-#include "net/instaweb/util/public/debug.h"
-#include "net/instaweb/util/public/shared_circular_buffer.h"
-#include "net/instaweb/util/public/string_util.h"
 #include "net/instaweb/public/version.h"
+#include "pagespeed/kernel/base/abstract_mutex.h"
+#include "pagespeed/kernel/base/debug.h"
 #include "pagespeed/kernel/base/posix_timer.h"
+#include "pagespeed/kernel/base/string_util.h"
 #include "pagespeed/kernel/base/time_util.h"
+#include "pagespeed/kernel/sharedmem/shared_circular_buffer.h"
 
 namespace {
 
@@ -56,11 +56,9 @@ extern "C" {
 
 namespace net_instaweb {
 
-NgxMessageHandler::NgxMessageHandler(AbstractMutex* mutex)
-    : mutex_(mutex),
-      buffer_(NULL),
+NgxMessageHandler::NgxMessageHandler(Timer* timer, AbstractMutex* mutex)
+    : SystemMessageHandler(timer, mutex),
       log_(NULL) {
-  SetPidString(static_cast<int64>(getpid()));
 }
 
 // Installs a signal handler for common crash signals, that tries to print
@@ -73,14 +71,6 @@ void NgxMessageHandler::InstallCrashHandler(ngx_log_t* log) {
   signal(SIGSEGV, signal_handler);
 }
 
-bool NgxMessageHandler::Dump(Writer* writer) {
-  // Can't dump before SharedCircularBuffer is set up.
-  if (buffer_ == NULL) {
-    return false;
-  }
-  return buffer_->Dump(writer, &handler_);
-}
-
 ngx_uint_t NgxMessageHandler::GetNgxLogLevel(MessageType type) {
   switch (type) {
     case kInfo:
@@ -98,53 +88,28 @@ ngx_uint_t NgxMessageHandler::GetNgxLogLevel(MessageType type) {
   return NGX_LOG_ALERT;
 }
 
-void NgxMessageHandler::set_buffer(SharedCircularBuffer* buff) {
-  ScopedMutex lock(mutex_.get());
-  buffer_ = buff;
-}
-
-void NgxMessageHandler::MessageVImpl(MessageType type, const char* msg,
-                                     va_list args) {
-  ngx_uint_t log_level = GetNgxLogLevel(type);
-  GoogleString formatted_message = Format(msg, args);
+void NgxMessageHandler::MessageSImpl(MessageType type,
+                                     const GoogleString& message) {
   if (log_ != NULL) {
+    ngx_uint_t log_level = GetNgxLogLevel(type);
     ngx_log_error(log_level, log_, 0/*ngx_err_t*/, "[%s %s] %s",
-                  kModuleName, kModPagespeedVersion, formatted_message.c_str());
+                  kModuleName, kModPagespeedVersion, message.c_str());
   } else {
-    GoogleMessageHandler::MessageVImpl(type, msg, args);
+    GoogleMessageHandler::MessageSImpl(type, message);
   }
-
   // Prepare a log message for the SharedCircularBuffer only.
-  // Prepend time and severity to message.
-  // Format is [time] [severity] [pid] message.
-  GoogleString message;
-  GoogleString time;
-  PosixTimer timer;
-  if (!ConvertTimeToString(timer.NowMs(), &time)) {
-    time = "?";
-  }
-  StrAppend(&message, "[", time, "] ",
-            "[", MessageTypeToString(type), "] ");
-  StrAppend(&message, pid_string_, " ", formatted_message, "\n");
-  {
-    ScopedMutex lock(mutex_.get());
-    if (buffer_ != NULL) {
-      buffer_->Write(message);
-    }
-  }
+  AddMessageToBuffer(type, message);
 }
 
-void NgxMessageHandler::FileMessageVImpl(MessageType type, const char* file,
-                                         int line, const char* msg,
-                                         va_list args) {
-  ngx_uint_t log_level = GetNgxLogLevel(type);
-  GoogleString formatted_message = Format(msg, args);
+void NgxMessageHandler::FileMessageSImpl(
+    MessageType type, const char* file, int line, const GoogleString& message) {
   if (log_ != NULL) {
+    ngx_uint_t log_level = GetNgxLogLevel(type);
     ngx_log_error(log_level, log_, 0/*ngx_err_t*/, "[%s %s] %s:%d:%s",
                   kModuleName, kModPagespeedVersion, file, line,
-                  formatted_message.c_str());
+                  message.c_str());
   } else {
-    GoogleMessageHandler::FileMessageVImpl(type, file, line, msg, args);
+    GoogleMessageHandler::FileMessageSImpl(type, file, line, message);
   }
 }
 

src/ngx_message_handler.h

@@ -18,65 +18,48 @@
 #define NGX_MESSAGE_HANDLER_H_
 
 extern "C" {
+  #include <ngx_auto_config.h>
+#if (NGX_THREADS)
+  #include <ngx_thread.h>
+#endif
   #include <ngx_core.h>
   #include <ngx_log.h>
 }
 
 #include <cstdarg>
 
-#include "net/instaweb/util/public/basictypes.h"
-#include "net/instaweb/util/public/google_message_handler.h"
-#include "net/instaweb/util/public/message_handler.h"
-#include "net/instaweb/util/public/scoped_ptr.h"
-#include "net/instaweb/util/public/string.h"
-#include "net/instaweb/util/public/string_util.h"
+#include "pagespeed/kernel/base/basictypes.h"
+#include "pagespeed/kernel/base/message_handler.h"
+#include "pagespeed/kernel/base/string.h"
+#include "pagespeed/kernel/base/string_util.h"
+#include "pagespeed/system/system_message_handler.h"
 
 namespace net_instaweb {
 
 class AbstractMutex;
-class SharedCircularBuffer;
 class Timer;
-class Writer;
 
 // Implementation of a message handler that uses ngx_log_error()
 // logging to emit messages, with a fallback to GoogleMessageHandler
-class NgxMessageHandler : public GoogleMessageHandler {
+class NgxMessageHandler : public SystemMessageHandler {
  public:
-  explicit NgxMessageHandler(AbstractMutex* mutex);
+  explicit NgxMessageHandler(Timer* timer, AbstractMutex* mutex);
 
   // Installs a signal handler for common crash signals that tries to print
   // out a backtrace.
   static void InstallCrashHandler(ngx_log_t* log);
 
-  // When NgxRewriteDriver instantiates the NgxMessageHandlers, the
-  // SharedCircularBuffer and ngx_log_t are not available yet. These
-  // will later be set in RootInit/Childinit
-  // Messages logged before that will be passed on to handler_;
-  void set_buffer(SharedCircularBuffer* buff);
   void set_log(ngx_log_t* log) { log_ = log; }
   ngx_log_t* log() { return log_; }
 
-  void SetPidString(const int64 pid) {
-    pid_string_ = StrCat("[", Integer64ToString(pid), "]");
-  }
-  // Dump contents of SharedCircularBuffer.
-  bool Dump(Writer* writer);
-
  protected:
-  virtual void MessageVImpl(MessageType type, const char* msg, va_list args);
+  virtual void MessageSImpl(MessageType type, const GoogleString& message);
 
-  virtual void FileMessageVImpl(MessageType type, const char* filename,
-                                int line, const char* msg, va_list args);
+  virtual void FileMessageSImpl(MessageType type, const char* file,
+                                int line, const GoogleString& message);
 
  private:
   ngx_uint_t GetNgxLogLevel(MessageType type);
-
-  scoped_ptr<AbstractMutex> mutex_;
-  GoogleString pid_string_;
-  // handler_ is used as a fallback when we can not use ngx_log_errort
-  // It's also used when calling Dump on the internal SharedCircularBuffer
-  GoogleMessageHandler handler_;
-  SharedCircularBuffer* buffer_;
   ngx_log_t* log_;
 
   DISALLOW_COPY_AND_ASSIGN(NgxMessageHandler);

src/ngx_pagespeed.h

@@ -34,8 +34,8 @@ extern "C" {
 }
 
 #include "base/logging.h"
-#include "net/instaweb/http/public/response_headers.h"
-#include "net/instaweb/util/public/string_util.h"
+#include "pagespeed/kernel/base/string_util.h"
+#include "pagespeed/kernel/http/response_headers.h"
 
 namespace net_instaweb {
 
@@ -84,15 +84,11 @@ enum PreserveCachingHeaders {
 typedef struct {
   NgxBaseFetch* base_fetch;
 
-  ngx_connection_t* pagespeed_connection;
   ngx_http_request_t* r;
 
   bool html_rewrite;
   bool in_place;
 
-  bool write_pending;
-  bool fetch_done;
-
   PreserveCachingHeaders preserve_caching_headers;
 
   // for html rewrite
@@ -107,8 +103,14 @@ typedef struct {
   // We need to remember the URL here as well since we may modify what NGX
   // gets by stripping our special query params and honoring X-Forwarded-Proto.
   GoogleString url_string;
+
+  // We need to remember if the upstream had headers_out->location set, because
+  // we should mirror that when we write it back. nginx may absolutify
+  // Location: headers that start with '/' without regarding X-Forwarded-Proto.
+  bool location_field_set;
 } ps_request_ctx_t;
 
+ps_request_ctx_t* ps_get_request_context(ngx_http_request_t* r);
 
 void copy_request_headers_from_ngx(const ngx_http_request_t* r,
                                    RequestHeaders* headers);
@@ -123,6 +125,12 @@ ngx_int_t copy_response_headers_to_ngx(
 
 StringPiece ps_determine_host(ngx_http_request_t* r);
 
+namespace ps_base_fetch {
+
+ngx_int_t ps_base_fetch_handler(ngx_http_request_t* r);
+
+}  // namespace ps_base_fetch
+
 }  // namespace net_instaweb
 
 #endif  // NGX_PAGESPEED_H_

src/ngx_rewrite_driver_factory.cc

@@ -26,30 +26,30 @@
 #include "ngx_server_context.h"
 #include "ngx_url_async_fetcher.h"
 
-#include "net/instaweb/http/public/content_type.h"
 #include "net/instaweb/http/public/rate_controller.h"
 #include "net/instaweb/http/public/rate_controlling_url_async_fetcher.h"
 #include "net/instaweb/http/public/wget_url_fetcher.h"
 #include "net/instaweb/rewriter/public/rewrite_driver.h"
 #include "net/instaweb/rewriter/public/rewrite_driver_factory.h"
 #include "net/instaweb/rewriter/public/server_context.h"
-#include "net/instaweb/system/public/in_place_resource_recorder.h"
-#include "net/instaweb/system/public/serf_url_async_fetcher.h"
-#include "net/instaweb/system/public/system_caches.h"
-#include "net/instaweb/system/public/system_rewrite_options.h"
-#include "net/instaweb/util/public/google_message_handler.h"
-#include "net/instaweb/util/public/null_shared_mem.h"
-#include "net/instaweb/util/public/posix_timer.h"
 #include "net/instaweb/util/public/property_cache.h"
-#include "net/instaweb/util/public/scheduler_thread.h"
-#include "net/instaweb/util/public/shared_circular_buffer.h"
-#include "net/instaweb/util/public/shared_mem_statistics.h"
-#include "net/instaweb/util/public/slow_worker.h"
-#include "net/instaweb/util/public/stdio_file_system.h"
-#include "net/instaweb/util/public/string.h"
-#include "net/instaweb/util/public/string_util.h"
-#include "net/instaweb/util/public/thread_system.h"
+#include "pagespeed/kernel/base/google_message_handler.h"
+#include "pagespeed/kernel/base/null_shared_mem.h"
+#include "pagespeed/kernel/base/posix_timer.h"
+#include "pagespeed/kernel/base/stdio_file_system.h"
+#include "pagespeed/kernel/base/string.h"
+#include "pagespeed/kernel/base/string_util.h"
+#include "pagespeed/kernel/base/thread_system.h"
+#include "pagespeed/kernel/http/content_type.h"
+#include "pagespeed/kernel/sharedmem/shared_circular_buffer.h"
+#include "pagespeed/kernel/sharedmem/shared_mem_statistics.h"
 #include "pagespeed/kernel/thread/pthread_shared_mem.h"
+#include "pagespeed/kernel/thread/scheduler_thread.h"
+#include "pagespeed/kernel/thread/slow_worker.h"
+#include "pagespeed/system/in_place_resource_recorder.h"
+#include "pagespeed/system/serf_url_async_fetcher.h"
+#include "pagespeed/system/system_caches.h"
+#include "pagespeed/system/system_rewrite_options.h"
 
 namespace net_instaweb {
 
@@ -71,17 +71,21 @@ NgxRewriteDriverFactory::NgxRewriteDriverFactory(
         NULL /* default shared memory runtime */, hostname, port),
       main_conf_(NULL),
       threads_started_(false),
-      use_per_vhost_statistics_(false),
-      ngx_message_handler_(new NgxMessageHandler(thread_system()->NewMutex())),
+      ngx_message_handler_(
+          new NgxMessageHandler(timer(), thread_system()->NewMutex())),
       ngx_html_parse_message_handler_(
-          new NgxMessageHandler(thread_system()->NewMutex())),
-      install_crash_handler_(false),
+          new NgxMessageHandler(timer(), thread_system()->NewMutex())),
       log_(NULL),
       resolver_timeout_(NGX_CONF_UNSET_MSEC),
       use_native_fetcher_(false),
+      // 100 Aligns to nginx's server-side default.
+      native_fetcher_max_keepalive_requests_(100),
       ngx_shared_circular_buffer_(NULL),
       hostname_(hostname.as_string()),
-      port_(port) {
+      port_(port),
+      process_script_variables_(false),
+      process_script_variables_set_(false),
+      shut_down_(false) {
   InitializeDefaultOptions();
   default_options()->set_beacon_url("/ngx_pagespeed_beacon");
   SystemRewriteOptions* system_options = dynamic_cast<SystemRewriteOptions*>(
@@ -111,6 +115,7 @@ UrlAsyncFetcher* NgxRewriteDriverFactory::AllocateFetcher(
         resolver_timeout_,
         config->blocking_fetch_timeout_ms(),
         resolver_,
+        native_fetcher_max_keepalive_requests_,
         thread_system(),
         message_handler());
     ngx_url_async_fetchers_.push_back(fetcher);
@@ -147,16 +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) {
-    if (!ngx_url_async_fetchers_[i]->Init()) {
-      return false;
-    }
-  }
-  return true;
-}
-
 bool NgxRewriteDriverFactory::CheckResolver() {
   if (use_native_fetcher_ && resolver_ == NULL) {
     return false;
@@ -182,6 +177,13 @@ ServerContext* NgxRewriteDriverFactory::NewServerContext() {
   return NULL;
 }
 
+void NgxRewriteDriverFactory::ShutDown() {
+  if (!shut_down_) {
+    shut_down_ = true;
+    SystemRewriteDriverFactory::ShutDown();
+  }
+}
+
 void NgxRewriteDriverFactory::ShutDownMessageHandlers() {
   ngx_message_handler_->set_buffer(NULL);
   ngx_html_parse_message_handler_->set_buffer(NULL);
@@ -206,9 +208,11 @@ void NgxRewriteDriverFactory::StartThreads() {
   threads_started_ = true;
 }
 
-void NgxRewriteDriverFactory::LoggingInit(ngx_log_t* log) {
+void NgxRewriteDriverFactory::LoggingInit(
+    ngx_log_t* log, bool may_install_crash_handler) {
+  log_ = log;
   net_instaweb::log_message_handler::Install(log);
-  if (install_crash_handler_) {
+  if (may_install_crash_handler && install_crash_handler()) {
     NgxMessageHandler::InstallCrashHandler(log);
   }
   ngx_message_handler_->set_log(log);
@@ -225,7 +229,7 @@ void NgxRewriteDriverFactory::SetCircularBuffer(
 void NgxRewriteDriverFactory::SetServerContextMessageHandler(
     ServerContext* server_context, ngx_log_t* log) {
   NgxMessageHandler* handler = new NgxMessageHandler(
-      thread_system()->NewMutex());
+      timer(), thread_system()->NewMutex());
   handler->set_log(log);
   // The ngx_shared_circular_buffer_ will be NULL if MessageBufferSize hasn't
   // been raised from its default of 0.

src/ngx_rewrite_driver_factory.h

@@ -20,6 +20,10 @@
 #define NGX_REWRITE_DRIVER_FACTORY_H_
 
 extern "C" {
+  #include <ngx_auto_config.h>
+#if (NGX_THREADS)
+  #include <ngx_thread.h>
+#endif
   #include <ngx_core.h>
   #include <ngx_http.h>
   #include <ngx_config.h>
@@ -28,13 +32,9 @@ extern "C" {
 
 #include <set>
 
-#include "net/instaweb/system/public/system_rewrite_driver_factory.h"
-#include "net/instaweb/util/public/md5_hasher.h"
-#include "net/instaweb/util/public/scoped_ptr.h"
-
-// TODO(oschaaf): We should reparent ApacheRewriteDriverFactory and
-// NgxRewriteDriverFactory to a new class OriginRewriteDriverFactory and factor
-// out as much as possible.
+#include "pagespeed/kernel/base/md5_hasher.h"
+#include "pagespeed/kernel/base/scoped_ptr.h"
+#include "pagespeed/system/system_rewrite_driver_factory.h"
 
 namespace net_instaweb {
 
@@ -66,7 +66,6 @@ class NgxRewriteDriverFactory : public SystemRewriteDriverFactory {
   // NgxRewriteOptions.
   virtual RewriteOptions* NewRewriteOptions();
   virtual ServerContext* NewDecodingServerContext();
-  bool InitNgxUrlAsyncFetchers();
   // Check resolver configured or not.
   bool CheckResolver();
 
@@ -76,6 +75,7 @@ class NgxRewriteDriverFactory : public SystemRewriteDriverFactory {
   static void InitStats(Statistics* statistics);
   NgxServerContext* MakeNgxServerContext(StringPiece hostname, int port);
   virtual ServerContext* NewServerContext();
+  virtual void ShutDown();
 
   // Starts pagespeed threads if they've not been started already.  Must be
   // called after the caller has finished any forking it intends to do.
@@ -92,18 +92,6 @@ class NgxRewriteDriverFactory : public SystemRewriteDriverFactory {
 
   void set_main_conf(NgxRewriteOptions* main_conf) {  main_conf_ = main_conf; }
 
-  bool use_per_vhost_statistics() const {
-    return use_per_vhost_statistics_;
-  }
-  void set_use_per_vhost_statistics(bool x) {
-    use_per_vhost_statistics_ = x;
-  }
-  bool install_crash_handler() const {
-    return install_crash_handler_;
-  }
-  void set_install_crash_handler(bool x) {
-    install_crash_handler_ = x;
-  }
   void set_resolver(ngx_resolver_t* resolver) {
     resolver_ = resolver;
   }
@@ -117,24 +105,31 @@ class NgxRewriteDriverFactory : public SystemRewriteDriverFactory {
   void set_use_native_fetcher(bool x) {
     use_native_fetcher_ = x;
   }
-  void set_rate_limit_background_fetches(bool x) {
-    rate_limit_background_fetches_ = x;
+  int native_fetcher_max_keepalive_requests() {
+    return native_fetcher_max_keepalive_requests_;
+  }
+  void set_native_fetcher_max_keepalive_requests(int x) {
+    native_fetcher_max_keepalive_requests_ = x;
+  }
+  bool process_script_variables() {
+    return process_script_variables_;
   }
 
-  // We use a beacon handler to collect data for critical images,
-  // css, etc., so filters should be configured accordingly.
-  //
-  // TODO(jefftk): move to SystemRewriteDriverFactory
-  virtual bool UseBeaconResultsInFilters() const {
-    return true;
-  }
-
-  void LoggingInit(ngx_log_t* log);
+  void LoggingInit(ngx_log_t* log, bool may_install_crash_handler);
 
   virtual void ShutDownMessageHandlers();
 
   virtual void SetCircularBuffer(SharedCircularBuffer* buffer);
 
+  bool SetProcessScriptVariables(bool process_script_variables) {
+    if (!process_script_variables_set_) {
+      process_script_variables_ = process_script_variables;
+      process_script_variables_set_ = true;
+      return true;
+    }
+    return false;
+  }
+
  private:
   Timer* timer_;
 
@@ -143,20 +138,16 @@ class NgxRewriteDriverFactory : public SystemRewriteDriverFactory {
   NgxRewriteOptions* main_conf_;
 
   bool threads_started_;
-  // If true, we'll have a separate statistics object for each vhost
-  // (along with a global aggregate), rather than just a single object
-  // aggregating all of them.
-  bool use_per_vhost_statistics_;
   NgxMessageHandler* ngx_message_handler_;
   NgxMessageHandler* ngx_html_parse_message_handler_;
-  bool install_crash_handler_;
 
   std::vector<NgxUrlAsyncFetcher*> ngx_url_async_fetchers_;
   ngx_log_t* log_;
   ngx_msec_t resolver_timeout_;
   ngx_resolver_t* resolver_;
   bool use_native_fetcher_;
-  bool rate_limit_background_fetches_;
+  int native_fetcher_max_keepalive_requests_;
+
   typedef std::set<NgxMessageHandler*> NgxMessageHandlerSet;
   NgxMessageHandlerSet server_context_message_handlers_;
 
@@ -166,6 +157,9 @@ class NgxRewriteDriverFactory : public SystemRewriteDriverFactory {
 
   GoogleString hostname_;
   int port_;
+  bool process_script_variables_;
+  bool process_script_variables_set_;
+  bool shut_down_;
 
   DISALLOW_COPY_AND_ASSIGN(NgxRewriteDriverFactory);
 };

src/ngx_rewrite_options.cc

@@ -30,8 +30,9 @@ extern "C" {
 #include "net/instaweb/public/version.h"
 #include "net/instaweb/rewriter/public/file_load_policy.h"
 #include "net/instaweb/rewriter/public/rewrite_options.h"
-#include "net/instaweb/system/public/system_caches.h"
-#include "net/instaweb/util/public/timer.h"
+#include "pagespeed/kernel/base/message_handler.h"
+#include "pagespeed/kernel/base/timer.h"
+#include "pagespeed/system/system_caches.h"
 
 namespace net_instaweb {
 
@@ -71,12 +72,14 @@ const char* const server_only_options[] = {
   "LoadFromFileMatch",
   "LoadFromFileRule",
   "LoadFromFileRuleMatch",
-  "UseNativeFetcher"
+  "UseNativeFetcher",
+  "NativeFetcherMaxKeepaliveRequests"
 };
 
 // Options that can only be used in the main (http) option scope.
 const char* const main_only_options[] = {
-  "UseNativeFetcher"
+  "UseNativeFetcher",
+  "NativeFetcherMaxKeepaliveRequests"
 };
 
 }  // namespace
@@ -97,6 +100,7 @@ NgxRewriteOptions::NgxRewriteOptions(ThreadSystem* thread_system)
 void NgxRewriteOptions::Init() {
   DCHECK(ngx_properties_ != NULL)
       << "Call NgxRewriteOptions::Initialize() before construction";
+  clear_inherited_scripts_ = false;
   InitializeOptions(ngx_properties_);
 }
 
@@ -104,23 +108,27 @@ void NgxRewriteOptions::AddProperties() {
   // Nginx-specific options.
   add_ngx_option(
       "", &NgxRewriteOptions::statistics_path_, "nsp", kStatisticsPath,
-      kServerScope, "Set the statistics path. Ex: /ngx_pagespeed_statistics");
+      kServerScope, "Set the statistics path. Ex: /ngx_pagespeed_statistics",
+      false);
   add_ngx_option(
       "", &NgxRewriteOptions::global_statistics_path_, "ngsp",
       kGlobalStatisticsPath, kProcessScope,
-      "Set the global statistics path. Ex: /ngx_pagespeed_global_statistics");
+      "Set the global statistics path. Ex: /ngx_pagespeed_global_statistics",
+      false);
   add_ngx_option(
       "", &NgxRewriteOptions::console_path_, "ncp", kConsolePath, kServerScope,
-      "Set the console path. Ex: /pagespeed_console");
+      "Set the console path. Ex: /pagespeed_console", false);
   add_ngx_option(
       "", &NgxRewriteOptions::messages_path_, "nmp", kMessagesPath,
-      kServerScope, "Set the messages path.  Ex: /ngx_pagespeed_message");
+      kServerScope, "Set the messages path.  Ex: /ngx_pagespeed_message",
+      false);
   add_ngx_option(
       "", &NgxRewriteOptions::admin_path_, "nap", kAdminPath,
-      kServerScope, "Set the admin path.  Ex: /pagespeed_admin");
+      kServerScope, "Set the admin path.  Ex: /pagespeed_admin", false);
   add_ngx_option(
       "", &NgxRewriteOptions::global_admin_path_, "ngap", kGlobalAdminPath,
-      kProcessScope, "Set the global admin path.  Ex: /pagespeed_global_admin");
+      kProcessScope, "Set the global admin path.  Ex: /pagespeed_global_admin",
+      false);
 
   MergeSubclassProperties(ngx_properties_);
 
@@ -250,7 +258,7 @@ const char* ps_error_string_for_option(
 const char* NgxRewriteOptions::ParseAndSetOptions(
     StringPiece* args, int n_args, ngx_pool_t* pool, MessageHandler* handler,
     NgxRewriteDriverFactory* driver_factory,
-    RewriteOptions::OptionScope scope) {
+    RewriteOptions::OptionScope scope, ngx_conf_t* cf, bool compile_scripts) {
   CHECK_GE(n_args, 1);
 
   StringPiece directive = args[0];
@@ -266,80 +274,145 @@ const char* NgxRewriteOptions::ParseAndSetOptions(
         pool, directive, "cannot be set at this scope.");
   }
 
+  ScriptLine* script_line;
+  script_line = NULL;
+  // Note that LoadFromFile should not be scriptable on wildcard hosts,
+  // as browsers might be able to manipulate its natural use-case: $http_host.
+  if (!StringCaseStartsWith(directive, "LoadFromFile") &&
+      !StringCaseEqual(directive, "EnableFilters") &&
+      !StringCaseEqual(directive, "DisableFilters") &&
+      !StringCaseEqual(directive, "DownstreamCachePurgeLocationPrefix") &&
+      !StringCaseEqual(directive, "DownstreamCachePurgeMethod") &&
+      !StringCaseEqual(directive,
+                       "DownstreamCacheRewrittenPercentageThreshold") &&
+      !StringCaseEqual(directive, "ShardDomain")){
+    compile_scripts = false;
+  }
+
+  if (n_args == 1 && StringCaseEqual(directive, "ClearInheritedScripts")) {
+    clear_inherited_scripts_ = true;
+    return NGX_CONF_OK;
+  }
+
+  if (compile_scripts) {
+    CHECK(cf != NULL);
+    int i;
+    // Skip the first arg which is always 'pagespeed'
+    for (i = 1; i < n_args; i++) {
+      ngx_str_t script_source;
+
+      script_source.len = args[i].as_string().length();
+      std::string tmp = args[i].as_string();
+      script_source.data = reinterpret_cast<u_char*>(
+          const_cast<char*>(tmp.c_str()));
+
+      if (ngx_http_script_variables_count(&script_source) > 0) {
+        ngx_http_script_compile_t* sc =
+            reinterpret_cast<ngx_http_script_compile_t*>(
+                ngx_pcalloc(cf->pool, sizeof(ngx_http_script_compile_t)));
+        sc->cf = cf;
+        sc->source = &script_source;
+        sc->lengths = reinterpret_cast<ngx_array_t**>(
+            ngx_pcalloc(cf->pool, sizeof(ngx_array_t*)));
+        sc->values = reinterpret_cast<ngx_array_t**>(
+            ngx_pcalloc(cf->pool, sizeof(ngx_array_t*)));
+        sc->variables = 1;
+        sc->complete_lengths = 1;
+        sc->complete_values = 1;
+        if (ngx_http_script_compile(sc) != NGX_OK) {
+          return ps_error_string_for_option(
+              pool, directive, "Failed to compile script variables");
+        } else {
+          if (script_line == NULL) {
+            script_line = new ScriptLine(args, n_args, scope);
+          }
+          script_line->AddScriptAndArgIndex(sc, i);
+        }
+      }
+    }
+
+    if (script_line != NULL) {
+      script_lines_.push_back(RefCountedPtr<ScriptLine>(script_line));
+      // We have found script variables in the current configuration line, and
+      // prepared the associated rewriteoptions for that.
+      // We will defer parsing, validation and processing of this line to
+      // request time. That means we are done handling this configuration line.
+      return NGX_CONF_OK;
+    }
+  }
+
   GoogleString msg;
   OptionSettingResult result;
   if (n_args == 1) {
     result = ParseAndSetOptions0(directive, &msg, handler);
   } else if (n_args == 2) {
     StringPiece arg = args[1];
-    // TODO(morlovich): Remove these special hacks, and handle these via
-    // ParseAndSetOptionFromEnum1.
-    if (IsDirective(directive, "UsePerVHostStatistics")) {
+    if (IsDirective(directive, "UseNativeFetcher")) {
       result = ParseAndSetOptionHelper<NgxRewriteDriverFactory>(
           arg, driver_factory,
-          &NgxRewriteDriverFactory::set_use_per_vhost_statistics);
-    } else if (IsDirective(directive, "InstallCrashHandler")) {
-      result = ParseAndSetOptionHelper<NgxRewriteDriverFactory>(
-          arg, driver_factory,
-          &NgxRewriteDriverFactory::set_install_crash_handler);
-    } else if (IsDirective(directive, "MessageBufferSize")) {
-      int message_buffer_size;
-      bool ok = StringToInt(arg.as_string(), &message_buffer_size);
-      if (ok && message_buffer_size >= 0) {
-        driver_factory->set_message_buffer_size(message_buffer_size);
+          &NgxRewriteDriverFactory::set_use_native_fetcher);
+    } else if (IsDirective(directive, "NativeFetcherMaxKeepaliveRequests")) {
+      int max_keepalive_requests;
+      if (StringToInt(arg, &max_keepalive_requests) &&
+          max_keepalive_requests > 0) {
+        driver_factory->set_native_fetcher_max_keepalive_requests(
+            max_keepalive_requests);
         result = RewriteOptions::kOptionOk;
       } else {
         result = RewriteOptions::kOptionValueInvalid;
       }
-    } else if (IsDirective(directive, "UseNativeFetcher")) {
-      result = ParseAndSetOptionHelper<NgxRewriteDriverFactory>(
-          arg, driver_factory,
-          &NgxRewriteDriverFactory::set_use_native_fetcher);
-    } else if (IsDirective(directive, "RateLimitBackgroundFetches")) {
-      result = ParseAndSetOptionHelper<NgxRewriteDriverFactory>(
-          arg, driver_factory,
-          &NgxRewriteDriverFactory::set_rate_limit_background_fetches);
-    } else if (IsDirective(directive, "ForceCaching")) {
-      result = ParseAndSetOptionHelper<SystemRewriteDriverFactory>(
-          arg, driver_factory,
-          &SystemRewriteDriverFactory::set_force_caching);
-    } else if (IsDirective(directive, "ListOutstandingUrlsOnError")) {
-      result = ParseAndSetOptionHelper<SystemRewriteDriverFactory>(
-          arg, driver_factory,
-          &SystemRewriteDriverFactory::list_outstanding_urls_on_error);
-    } else if (IsDirective(directive, "TrackOriginalContentLength")) {
-      result = ParseAndSetOptionHelper<SystemRewriteDriverFactory>(
-          arg, driver_factory,
-          &SystemRewriteDriverFactory::set_track_original_content_length);
-    } else if (IsDirective(directive, "StaticAssetPrefix")) {
-      driver_factory->set_static_asset_prefix(arg);
-      result = RewriteOptions::kOptionOk;
-    } else {
-      result = ParseAndSetOptionFromName1(directive, arg, &msg, handler);
-    }
-  } else if (n_args == 3) {
-    // Short-term special handling, until this moves to common code.
-    // TODO(morlovich): Clean this up.
-    if (StringCaseEqual(directive, "CreateSharedMemoryMetadataCache")) {
-      int64 kb = 0;
-      if (!StringToInt64(args[2], &kb) || kb < 0) {
-        result = RewriteOptions::kOptionValueInvalid;
-        msg = "size_kb must be a positive 64-bit integer";
+    } else if (StringCaseEqual("ProcessScriptVariables", args[0])) {
+      if (scope == RewriteOptions::kProcessScopeStrict) {
+        if (StringCaseEqual(arg, "on")) {
+          if (driver_factory->SetProcessScriptVariables(true)) {
+            result = RewriteOptions::kOptionOk;
+          } else {
+            return const_cast<char*>(
+                "pagespeed ProcessScriptVariables: can only be set once");
+          }
+        } else if (StringCaseEqual(arg, "off")) {
+          if (driver_factory->SetProcessScriptVariables(false)) {
+            result = RewriteOptions::kOptionOk;
+          } else {
+            return const_cast<char*>(
+                "pagespeed ProcessScriptVariables: can only be set once");
+          }
+        } else {
+          return const_cast<char*>(
+              "pagespeed ProcessScriptVariables: invalid value");
+        }
       } else {
-        bool ok = driver_factory->caches()->CreateShmMetadataCache(
-            args[1].as_string(), kb, &msg);
-        result = ok ? kOptionOk : kOptionValueInvalid;
+        return const_cast<char*>(
+            "ProcessScriptVariables is only allowed at the top level");
       }
     } else {
-      result = ParseAndSetOptionFromName2(directive, args[1], args[2],
-                                          &msg, handler);
+      result = ParseAndSetOptionFromName1(directive, arg, &msg, handler);
+      if (result == RewriteOptions::kOptionNameUnknown) {
+        result = driver_factory->ParseAndSetOption1(
+            directive,
+            arg,
+            scope >= RewriteOptions::kProcessScope,
+            &msg,
+            handler);
+      }
+    }
+  } else if (n_args == 3) {
+    result = ParseAndSetOptionFromName2(directive, args[1], args[2],
+                                        &msg, handler);
+    if (result == RewriteOptions::kOptionNameUnknown) {
+      result = driver_factory->ParseAndSetOption2(
+          directive,
+          args[1],
+          args[2],
+          scope >= RewriteOptions::kProcessScope,
+          &msg,
+          handler);
     }
   } else if (n_args == 4) {
     result = ParseAndSetOptionFromName3(
         directive, args[1], args[2], args[3], &msg, handler);
   } else {
-    return ps_error_string_for_option(
-        pool, directive, "not recognized or too many arguments");
+    result = RewriteOptions::kOptionNameUnknown;
   }
 
   switch (result) {
@@ -361,9 +434,84 @@ const char* NgxRewriteOptions::ParseAndSetOptions(
   return NULL;
 }
 
+// Execute all entries in the script_lines vector, and hand the result off to
+// ParseAndSetOptions to obtain the final option values.
+bool NgxRewriteOptions::ExecuteScriptVariables(
+    ngx_http_request_t* r, MessageHandler* handler,
+    NgxRewriteDriverFactory* driver_factory) {
+  bool script_error = false;
+
+  if (script_lines_.size() > 0) {
+    std::vector<RefCountedPtr<ScriptLine> >::iterator it;
+    for (it = script_lines_.begin() ; it != script_lines_.end(); ++it) {
+      ScriptLine* script_line = it->get();
+      StringPiece args[NGX_PAGESPEED_MAX_ARGS];
+      std::vector<ScriptArgIndex*>::iterator cs_it;
+      int i;
+
+      for (i = 0; i < script_line->n_args(); i++) {
+        args[i] = script_line->args()[i];
+      }
+
+      for (cs_it = script_line->data().begin();
+           cs_it != script_line->data().end(); cs_it++) {
+        ngx_http_script_compile_t* script;
+        ngx_array_t* values;
+        ngx_array_t* lengths;
+        ngx_str_t value;
+
+        script = (*cs_it)->script();
+        lengths = *script->lengths;
+        values = *script->values;
+
+        if (ngx_http_script_run(r, &value, lengths->elts, 0, values->elts)
+            == NULL) {
+          handler->Message(kError, "ngx_http_script_run error");
+          script_error = true;
+          break;
+        } else  {
+          args[(*cs_it)->index()] = str_to_string_piece(value);
+        }
+      }
+
+      const char* status = ParseAndSetOptions(args, script_line->n_args(),
+          r->pool, handler, driver_factory, script_line->scope(), NULL /*cf*/,
+          false /*compile scripts*/);
+
+      if (status != NULL) {
+        script_error = true;
+        handler->Message(kWarning,
+            "Error setting option value from script: '%s'", status);
+        break;
+      }
+    }
+  }
+
+  if (script_error) {
+    handler->Message(kWarning,
+        "Script error(s) in configuration, disabling optimization");
+    set_enabled(RewriteOptions::kEnabledOff);
+    return false;
+  }
+
+  return true;
+}
+
+void NgxRewriteOptions::CopyScriptLinesTo(
+    NgxRewriteOptions* destination) const {
+  destination->script_lines_ = script_lines_;
+}
+
+void NgxRewriteOptions::AppendScriptLinesTo(
+    NgxRewriteOptions* destination) const {
+  destination->script_lines_.insert(destination->script_lines_.end(),
+                                    script_lines_.begin(), script_lines_.end());
+}
+
 NgxRewriteOptions* NgxRewriteOptions::Clone() const {
   NgxRewriteOptions* options = new NgxRewriteOptions(
       StrCat("cloned from ", description()), thread_system());
+  this->CopyScriptLinesTo(options);
   options->Merge(*this);
   return options;
 }

src/ngx_rewrite_options.h

@@ -27,13 +27,81 @@ extern "C" {
   #include <ngx_http.h>
 }
 
+#include <vector>
+
 #include "net/instaweb/rewriter/public/rewrite_options.h"
-#include "net/instaweb/system/public/system_rewrite_options.h"
+#include "pagespeed/kernel/base/message_handler.h"
+#include "pagespeed/kernel/base/ref_counted_ptr.h"
+#include "pagespeed/kernel/base/stl_util.h"          // for STLDeleteElements
+#include "pagespeed/system/system_rewrite_options.h"
+
+#define NGX_PAGESPEED_MAX_ARGS 10
 
 namespace net_instaweb {
 
 class NgxRewriteDriverFactory;
 
+class ScriptArgIndex {
+ public:
+  explicit ScriptArgIndex(ngx_http_script_compile_t* script, int index)
+    : script_(script), index_(index) {
+      CHECK(script != NULL);
+      CHECK(index > 0 && index < NGX_PAGESPEED_MAX_ARGS);
+  }
+
+  virtual ~ScriptArgIndex() {}
+
+  ngx_http_script_compile_t* script() { return script_; }
+  int index() { return index_; }
+
+ private:
+  // Not owned.
+  ngx_http_script_compile_t* script_;
+  int index_;
+};
+
+// Refcounted, because the ScriptArgIndexes inside data_ can be shared between
+// different rewriteoptions.
+class ScriptLine : public RefCounted<ScriptLine> {
+ public:
+  explicit ScriptLine(StringPiece* args, int n_args,
+                      RewriteOptions::OptionScope scope)
+    : n_args_(n_args),
+      scope_(scope) {
+
+      for (int i = 0; i < n_args; i++) {
+        args_[i] = args[i];
+      }
+  }
+
+  virtual ~ScriptLine() {
+    STLDeleteElements(&data_);
+    data_.clear();
+  }
+
+  void AddScriptAndArgIndex(ngx_http_script_compile_t* script,
+                            int script_index) {
+    CHECK(script != NULL);
+    CHECK(script_index <  NGX_PAGESPEED_MAX_ARGS);
+    data_.push_back(new ScriptArgIndex(script, script_index));
+  }
+
+  int n_args() { return n_args_;}
+  StringPiece* args() { return args_;}
+  RewriteOptions::OptionScope scope() { return scope_; }
+  std::vector<ScriptArgIndex*>& data() {
+    return data_;
+  }
+
+ private:
+  StringPiece args_[NGX_PAGESPEED_MAX_ARGS];
+  int n_args_;
+  RewriteOptions::OptionScope scope_;
+  std::vector<ScriptArgIndex*> data_;
+
+  DISALLOW_COPY_AND_ASSIGN(ScriptLine);
+};
+
 class NgxRewriteOptions : public SystemRewriteOptions {
  public:
   // See rewrite_options::Initialize and ::Terminate
@@ -56,9 +124,19 @@ class NgxRewriteOptions : public SystemRewriteOptions {
   // on failure.
   //
   // pool is a memory pool for allocating error strings.
+  // cf is only required when compile_scripts is true
+  // when compile_scripts is true, the rewrite_options will be prepared
+  // for replacing any script $variables encountered in args. when false,
+  // script variables will be substituted using the prepared rewrite options.
   const char* ParseAndSetOptions(
       StringPiece* args, int n_args, ngx_pool_t* pool, MessageHandler* handler,
-      NgxRewriteDriverFactory* driver_factory, OptionScope scope);
+      NgxRewriteDriverFactory* driver_factory, OptionScope scope,
+      ngx_conf_t* cf, bool compile_scripts);
+  bool ExecuteScriptVariables(
+      ngx_http_request_t* r, MessageHandler* handler,
+      NgxRewriteDriverFactory* driver_factory);
+  void CopyScriptLinesTo(NgxRewriteOptions* destination) const;
+  void AppendScriptLinesTo(NgxRewriteOptions* destination) const;
 
   // Make an identical copy of these options and return it.
   virtual NgxRewriteOptions* Clone() const;
@@ -86,6 +164,12 @@ class NgxRewriteOptions : public SystemRewriteOptions {
   const GoogleString& global_admin_path() const {
     return global_admin_path_.value();
   }
+  const std::vector<RefCountedPtr<ScriptLine> >& script_lines() const {
+    return script_lines_;
+  }
+  const bool& clear_inherited_scripts() const {
+    return clear_inherited_scripts_;
+  }
 
  private:
   // Helper methods for ParseAndSetOptions().  Each can:
@@ -129,9 +213,10 @@ class NgxRewriteOptions : public SystemRewriteOptions {
                              const char* id,
                              StringPiece option_name,
                              OptionScope scope,
-                             const char* help) {
+                             const char* help,
+                             bool safe_to_print) {
     AddProperty(default_value, offset, id, option_name, scope, help,
-                ngx_properties_);
+                safe_to_print, ngx_properties_);
   }
 
   Option<GoogleString> statistics_path_;
@@ -141,6 +226,9 @@ class NgxRewriteOptions : public SystemRewriteOptions {
   Option<GoogleString> admin_path_;
   Option<GoogleString> global_admin_path_;
 
+  bool clear_inherited_scripts_;
+  std::vector<RefCountedPtr<ScriptLine> > script_lines_;
+
   // Helper for ParseAndSetOptions.  Returns whether the two directives equal,
   // ignoring case.
   bool IsDirective(StringPiece config_directive, StringPiece compare_directive);

src/ngx_server_context.cc

@@ -27,9 +27,9 @@ extern "C" {
 #include "ngx_rewrite_driver_factory.h"
 #include "ngx_rewrite_options.h"
 #include "net/instaweb/rewriter/public/rewrite_driver.h"
-#include "net/instaweb/system/public/add_headers_fetcher.h"
-#include "net/instaweb/system/public/loopback_route_fetcher.h"
-#include "net/instaweb/system/public/system_request_context.h"
+#include "pagespeed/system/add_headers_fetcher.h"
+#include "pagespeed/system/loopback_route_fetcher.h"
+#include "pagespeed/system/system_request_context.h"
 
 namespace net_instaweb {
 
@@ -48,7 +48,7 @@ SystemRequestContext* NgxServerContext::NewRequestContext(
     ngx_http_request_t* r) {
   // Based on ngx_http_variable_server_port.
   bool port_set = false;
-  int local_port;
+  int local_port = 0;
 #if (NGX_HAVE_INET6)
   if (r->connection->local_sockaddr->sa_family == AF_INET6) {
     local_port = ntohs(reinterpret_cast<struct sockaddr_in6*>(
@@ -77,4 +77,9 @@ SystemRequestContext* NgxServerContext::NewRequestContext(
                                   str_to_string_piece(local_ip));
 }
 
+GoogleString NgxServerContext::FormatOption(StringPiece option_name,
+                                            StringPiece args) {
+  return StrCat("pagespeed ", option_name, " ", args, ";");
+}
+
 }  // namespace net_instaweb

src/ngx_server_context.h

@@ -22,7 +22,7 @@
 #define NGX_SERVER_CONTEXT_H_
 
 #include "ngx_message_handler.h"
-#include "net/instaweb/system/public/system_server_context.h"
+#include "pagespeed/system/system_server_context.h"
 
 extern "C" {
 #include <ngx_http.h>
@@ -40,8 +40,9 @@ class NgxServerContext : public SystemServerContext {
       NgxRewriteDriverFactory* factory, StringPiece hostname, int port);
   virtual ~NgxServerContext();
 
-  // We expect to use ProxyFetch with HTML.
-  virtual bool ProxiesHtml() const { return true; }
+  // We don't allow ProxyFetch to fetch HTML via MapProxyDomain. We will call
+  // set_trusted_input() on any ProxyFetches we use to transform internal HTML.
+  virtual bool ProxiesHtml() const { return false; }
 
   // Call only when you need an NgxRewriteOptions.  If you don't need
   // nginx-specific behavior, call global_options() instead which doesn't
@@ -55,6 +56,8 @@ class NgxServerContext : public SystemServerContext {
     return dynamic_cast<NgxMessageHandler*>(message_handler());
   }
 
+  virtual GoogleString FormatOption(StringPiece option_name, StringPiece args);
+
  private:
   NgxRewriteDriverFactory* ngx_factory_;
 

src/ngx_url_async_fetcher.cc

@@ -31,22 +31,22 @@ extern "C" {
 #include <map>
 #include <set>
 
-#include "net/instaweb/util/public/basictypes.h"
 #include "net/instaweb/http/public/async_fetch.h"
 #include "net/instaweb/http/public/inflating_fetch.h"
-#include "net/instaweb/http/public/request_headers.h"
-#include "net/instaweb/http/public/response_headers.h"
-#include "net/instaweb/http/public/response_headers_parser.h"
 #include "net/instaweb/public/version.h"
-#include "net/instaweb/util/public/condvar.h"
-#include "net/instaweb/util/public/message_handler.h"
-#include "net/instaweb/util/public/pool.h"
-#include "net/instaweb/util/public/pool_element.h"
-#include "net/instaweb/util/public/statistics.h"
-#include "net/instaweb/util/public/string_util.h"
-#include "net/instaweb/util/public/thread_system.h"
-#include "net/instaweb/util/public/timer.h"
-#include "net/instaweb/util/public/writer.h"
+#include "pagespeed/kernel/base/basictypes.h"
+#include "pagespeed/kernel/base/condvar.h"
+#include "pagespeed/kernel/base/message_handler.h"
+#include "pagespeed/kernel/base/pool.h"
+#include "pagespeed/kernel/base/pool_element.h"
+#include "pagespeed/kernel/base/statistics.h"
+#include "pagespeed/kernel/base/string_util.h"
+#include "pagespeed/kernel/base/thread_system.h"
+#include "pagespeed/kernel/base/timer.h"
+#include "pagespeed/kernel/base/writer.h"
+#include "pagespeed/kernel/http/request_headers.h"
+#include "pagespeed/kernel/http/response_headers.h"
+#include "pagespeed/kernel/http/response_headers_parser.h"
 
 namespace net_instaweb {
 
@@ -55,6 +55,7 @@ namespace net_instaweb {
                                          ngx_msec_t resolver_timeout,
                                          ngx_msec_t fetch_timeout,
                                          ngx_resolver_t* resolver,
+                                         int max_keepalive_requests,
                                          ThreadSystem* thread_system,
                                          MessageHandler* handler)
     : fetchers_count_(0),
@@ -63,7 +64,9 @@ namespace net_instaweb {
       byte_count_(0),
       thread_system_(thread_system),
       message_handler_(handler),
-      mutex_(NULL) {
+      mutex_(NULL),
+      max_keepalive_requests_(max_keepalive_requests),
+      event_connection_(NULL) {
     resolver_timeout_ = resolver_timeout;
     fetch_timeout_ = fetch_timeout;
     ngx_memzero(&proxy_, sizeof(proxy_));
@@ -74,12 +77,17 @@ namespace net_instaweb {
     mutex_ = thread_system_->NewMutex();
     log_ = log;
     pool_ = NULL;
-    command_connection_ = NULL;
-    pipe_fd_ = -1;
     resolver_ = resolver;
+    // If init fails, set shutdown_ so no fetches will be attempted.
+    if (!Init(const_cast<ngx_cycle_t*>(ngx_cycle))) {
+      shutdown_ = true;
+      message_handler_->Message(
+          kError, "NgxUrlAsyncFetcher failed to init, fetching disabled.");
+    }
   }
 
   NgxUrlAsyncFetcher::~NgxUrlAsyncFetcher() {
+    DCHECK(shutdown_)  << "Shut down before destructing NgxUrlAsyncFetcher.";
     message_handler_->Message(
         kInfo,
         "Destruct NgxUrlAsyncFetcher with [%d] active fetchers",
@@ -87,19 +95,12 @@ namespace net_instaweb {
 
     CancelActiveFetches();
     active_fetches_.DeleteAll();
+    NgxConnection::Terminate();
 
     if (pool_ != NULL) {
       ngx_destroy_pool(pool_);
       pool_ = NULL;
     }
-    if (command_connection_ != NULL) {
-      ngx_close_connection(command_connection_);
-      command_connection_ = NULL;
-    }
-    if (pipe_fd_ != -1) {
-      close(pipe_fd_);
-      pipe_fd_ = -1;
-    }
     if (mutex_ != NULL) {
       delete mutex_;
       mutex_ = NULL;
@@ -150,9 +151,13 @@ namespace net_instaweb {
 
   // Create the pool for fetcher, create the pipe, add the read event for main
   // thread. It should be called in the worker process.
-  bool NgxUrlAsyncFetcher::Init() {
-    log_ = ngx_cycle->log;
-
+  bool NgxUrlAsyncFetcher::Init(ngx_cycle_t* cycle) {
+    log_ = cycle->log;
+    CHECK(event_connection_ == NULL) << "event connection already set";
+    event_connection_ = new NgxEventConnection(ReadCallback);
+    if (!event_connection_->Init(cycle)) {
+      return false;
+    }
     if (pool_ == NULL) {
       pool_ = ngx_create_pool(4096, log_);
       if (pool_ == NULL) {
@@ -162,41 +167,6 @@ namespace net_instaweb {
       }
     }
 
-    int pipe_fds[2];
-    int rc = pipe(pipe_fds);
-    if (rc != 0) {
-      ngx_log_error(NGX_LOG_ERR, log_, 0, "pipe() failed");
-      return false;
-    }
-    if (ngx_nonblocking(pipe_fds[0]) == -1) {
-      ngx_log_error(NGX_LOG_ERR, log_, 0, "nonblocking pipe[0] failed");
-      return false;
-    }
-    if (ngx_nonblocking(pipe_fds[1]) == -1) {
-      ngx_log_error(NGX_LOG_ERR, log_, 0, "nonblocking pipe[1] failed");
-      return false;
-    }
-
-    pipe_fd_ = pipe_fds[1];
-    command_connection_ = ngx_get_connection(pipe_fds[0], log_);
-    if (command_connection_ == NULL) {
-      close(pipe_fds[1]);
-      close(pipe_fds[0]);
-      pipe_fd_ = -1;
-      return false;
-    }
-
-    command_connection_->recv = ngx_recv;
-    command_connection_->send = ngx_send;
-    command_connection_->recv_chain = ngx_recv_chain;
-    command_connection_->send_chain = ngx_send_chain;
-    command_connection_->log = log_;
-    command_connection_->read->log = log_;
-    command_connection_->write->log = log_;
-    command_connection_->data = this;
-    command_connection_->read->handler = CommandHandler;
-    ngx_add_event(command_connection_->read, NGX_READ_EVENT, 0);
-
     if (proxy_.url.len == 0) {
       return true;
     }
@@ -212,8 +182,29 @@ namespace net_instaweb {
   }
 
   void NgxUrlAsyncFetcher::ShutDown() {
-      shutdown_ = true;
-      SendCmd('S');
+    shutdown_ = true;
+    if (!pending_fetches_.empty()) {
+      for (Pool<NgxFetch>::iterator p = pending_fetches_.begin(),
+           e = pending_fetches_.end(); p != e; p++) {
+        NgxFetch* fetch = *p;
+        fetch->CallbackDone(false);
+      }
+      pending_fetches_.DeleteAll();
+    }
+
+    if (!active_fetches_.empty()) {
+      for (Pool<NgxFetch>::iterator p = active_fetches_.begin(),
+           e = active_fetches_.end(); p != e; p++) {
+        NgxFetch* fetch = *p;
+        fetch->CallbackDone(false);
+      }
+      active_fetches_.Clear();
+    }
+    if (event_connection_ != NULL) {
+      event_connection_->Shutdown();
+      delete event_connection_;
+      event_connection_ = NULL;
+    }
   }
 
   // It's called in the rewrite thread. All the fetches are started at
@@ -221,92 +212,46 @@ 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, fetch_timeout_, log_);
+          message_handler, log_);
     ScopedMutex lock(mutex_);
     pending_fetches_.Add(fetch);
-    SendCmd('F');
-  }
 
-  // send command to nginx main thread
-  // 'F' : start a fetch
-  // 'S' : shutdown the fetcher
-  bool NgxUrlAsyncFetcher::SendCmd(const char command) {
-    int rc;
-    while (true) {
-      rc = write(pipe_fd_, &command, 1);
-      if (rc == 1) {
-        return true;
-      } else if (errno == EINTR || errno == EAGAIN || errno == EWOULDBLOCK) {
-        // TODO(junmin): It's rare. But it need be fixed.
-      } else {
-        return false;
-      }
-    }
-    return true;
+    // TODO(oschaaf): thread safety on written vs shutdown.
+    // It is possible that shutdown() is called after writing an event? In that
+    // case, this could (rarely) fail when it shouldn't.
+    bool written = event_connection_->WriteEvent(this);
+    CHECK(written || shutdown_) << "NgxUrlAsyncFetcher: event write failure";
   }
 
   // This is the read event which is called in the main thread.
   // It will do the real work. Add the work event and start the fetch.
-  void NgxUrlAsyncFetcher::CommandHandler(ngx_event_t *cmdev) {
-    char command;
-    int rc;
-    ngx_connection_t* c = static_cast<ngx_connection_t*>(cmdev->data);
-    NgxUrlAsyncFetcher* fetcher = static_cast<NgxUrlAsyncFetcher*>(c->data);
-    do {
-      rc = read(c->fd, &command, 1);
-    } while (rc == -1 && errno == EINTR);
+  void NgxUrlAsyncFetcher::ReadCallback(const ps_event_data& data) {
+    std::vector<NgxFetch*> to_start;
+    NgxUrlAsyncFetcher* fetcher = reinterpret_cast<NgxUrlAsyncFetcher*>(
+      data.sender);
 
-    CHECK(rc == -1 || rc == 0 || rc == 1);
+    fetcher->mutex_->Lock();
+    fetcher->completed_fetches_.DeleteAll();
 
-    if (rc == -1 || rc == 0) {
-      // EAGAIN
-      return;
+    for (Pool<NgxFetch>::iterator p = fetcher->pending_fetches_.begin(),
+             e = fetcher->pending_fetches_.end(); p != e; p++) {
+      NgxFetch* fetch = *p;
+      to_start.push_back(fetch);
     }
 
-    std::vector<NgxFetch*> to_start;
+    fetcher->pending_fetches_.Clear();
+    fetcher->mutex_->Unlock();
 
-    switch (command) {
-      // All the new fetches are appended in the pending_fetches.
-      // Start all these fetches.
-      case 'F':
-        fetcher->mutex_->Lock();
-        fetcher->completed_fetches_.DeleteAll();
-        for (Pool<NgxFetch>::iterator p = fetcher->pending_fetches_.begin(),
-                 e = fetcher->pending_fetches_.end(); p != e; p++) {
-          NgxFetch* fetch = *p;
-          to_start.push_back(fetch);
-        }
-
-        fetcher->pending_fetches_.Clear();
-        fetcher->mutex_->Unlock();
-
-        for (size_t i = 0; i < to_start.size(); i++) {
-          fetcher->StartFetch(to_start[i]);
-        }
-        CHECK(ngx_handle_read_event(cmdev, 0) == NGX_OK);
-        break;
-
-      // Shutdown all the fetches.
-      case 'S':
-        if (!fetcher->pending_fetches_.empty()) {
-          fetcher->pending_fetches_.DeleteAll();
-        }
-
-        if (!fetcher->active_fetches_.empty()) {
-          for (Pool<NgxFetch>::iterator p = fetcher->active_fetches_.begin(),
-               e = fetcher->active_fetches_.end(); p != e; p++) {
-            NgxFetch* fetch = *p;
-            fetch->CallbackDone(false);
-          }
-          fetcher->active_fetches_.Clear();
-        }
-        CHECK(ngx_del_event(cmdev, NGX_READ_EVENT, 0) == NGX_OK);
-        break;
-
-      default:
-        break;
+    for (size_t i = 0; i < to_start.size(); i++) {
+      fetcher->StartFetch(to_start[i]);
     }
 
     return;

src/ngx_url_async_fetcher.h

@@ -33,11 +33,14 @@ extern "C" {
 }
 
 #include <vector>
+
+#include "ngx_event_connection.h"
+
 #include "net/instaweb/http/public/url_async_fetcher.h"
-#include "net/instaweb/util/public/basictypes.h"
-#include "net/instaweb/util/public/pool.h"
-#include "net/instaweb/util/public/string.h"
-#include "net/instaweb/util/public/thread_system.h"
+#include "pagespeed/kernel/base/basictypes.h"
+#include "pagespeed/kernel/base/pool.h"
+#include "pagespeed/kernel/base/string.h"
+#include "pagespeed/kernel/base/thread_system.h"
 
 
 namespace net_instaweb {
@@ -53,27 +56,27 @@ class NgxUrlAsyncFetcher : public UrlAsyncFetcher {
   NgxUrlAsyncFetcher(
       const char* proxy, ngx_log_t* log, ngx_msec_t resolver_timeout,
       ngx_msec_t fetch_timeout, ngx_resolver_t* resolver,
-      ThreadSystem* thread_system, MessageHandler* handler);
+      int max_keepalive_requests, ThreadSystem* thread_system,
+      MessageHandler* handler);
 
   ~NgxUrlAsyncFetcher();
 
   // It should be called in the module init_process callback function. Do some
   // intializations which can't be done in the master process
-  bool Init();
+  bool Init(ngx_cycle_t* cycle);
 
   // shutdown all the fetches.
   virtual void ShutDown();
 
+  // the read handler in the main thread
+  static void ReadCallback(const ps_event_data& data);
+
   virtual bool SupportsHttps() const { return false; }
 
   virtual void Fetch(const GoogleString& url,
                      MessageHandler* message_handler,
                      AsyncFetch* callback);
 
-  // send the command from the current thread to main thread
-  bool SendCmd(const char command);
-  // the read handler in the main thread
-  static void CommandHandler(ngx_event_t* cmdev);
   bool StartFetch(NgxFetch* fetch);
 
   // Remove the completed fetch from the active fetch set, and put it into a
@@ -136,12 +139,13 @@ class NgxUrlAsyncFetcher : public UrlAsyncFetcher {
 
   ngx_pool_t* pool_;
   ngx_log_t* log_;
-  ngx_connection_t* command_connection_;  // the command pipe
-  int pipe_fd_;  // the write pipe end
   ngx_resolver_t* resolver_;
+  int max_keepalive_requests_;
   ngx_msec_t resolver_timeout_;
   ngx_msec_t fetch_timeout_;
 
+  NgxEventConnection* event_connection_;
+
   DISALLOW_COPY_AND_ASSIGN(NgxUrlAsyncFetcher);
 };
 

test/run_tests.sh

@@ -23,9 +23,9 @@
 # Exits with status 2 if command line args are wrong.
 #
 # Usage:
-#   ./run_tests.sh primary_port secondary_port mod_pagespeed_dir
+#   ./run_tests.sh primary_port secondary_port mod_pagespeed_dir pagespeed_test_host
 # Example:
-#   ./run_tests.sh 8050 8051 /path/to/mod_pagespeed
+#   ./run_tests.sh 8050 8051 /path/to/mod_pagespeed www.modpagespeed.com
 #
 
 # Normally we test only with the native fetcher off.  Set
@@ -43,7 +43,7 @@ RUN_TESTS=${RUN_TESTS:-true}
 # true.
 USE_VALGRIND=${USE_VALGRIND:-false}
 
-if [ "$#" -ne 4 ] ; then
+if [ "$#" -ne 5 ] ; then
   echo "Usage: $0 primary_port secondary_port mod_pagespeed_dir"
   echo "  nginx_executable"
   exit 2
@@ -53,6 +53,14 @@ PRIMARY_PORT="$1"
 SECONDARY_PORT="$2"
 MOD_PAGESPEED_DIR="$3"
 NGINX_EXECUTABLE="$4"
+PAGESPEED_TEST_HOST="$5"
+RCPORT1=9991
+RCPORT2=9992
+RCPORT3=9993
+RCPORT4=9994
+RCPORT5=9995
+RCPORT6=9996
+RCPORT7=9997
 
 this_dir="$( cd $(dirname "$0") && pwd)"
 
@@ -62,7 +70,15 @@ function run_test_checking_failure() {
     SECONDARY_PORT="$SECONDARY_PORT" \
     MOD_PAGESPEED_DIR="$MOD_PAGESPEED_DIR" \
     NGINX_EXECUTABLE="$NGINX_EXECUTABLE" \
+    PAGESPEED_TEST_HOST="$PAGESPEED_TEST_HOST" \
     RUN_TESTS="$RUN_TESTS" \
+    RCPORT1="$RCPORT1" \
+    RCPORT2="$RCPORT2" \
+    RCPORT3="$RCPORT3" \
+    RCPORT4="$RCPORT4" \
+    RCPORT5="$RCPORT5" \
+    RCPORT6="$RCPORT6" \
+    RCPORT7="$RCPORT7" \
     bash "$this_dir/nginx_system_test.sh"
   STATUS=$?
   echo "With $@ setup."
@@ -73,6 +89,9 @@ function run_test_checking_failure() {
     3)
       return  # Only expected failures.
       ;;
+    4)
+      return  # Return passing error code when running manually.
+      ;;
     *)
       exit 1  # Real failure.
   esac