Bug 1024015 - Only enable HTTP response timeout when TCP Keepalives are disabled for HTTP r=mcmanus
authorSteve Workman <sworkman@mozilla.com>
Thu, 26 Jun 2014 11:03:45 -0700
changeset 191261 2700ebff7d64911ec97a62668508af24e3e5bdc6
parent 191260 6f2c1e191d9decba8f2e70df1d3ef677b5455863
child 191262 cc82b52c9a7fefca023a4e761a2bbf0db268c730
push id27037
push userkwierso@gmail.com
push dateSat, 28 Jun 2014 00:41:04 +0000
treeherdermozilla-central@286b6cb59c3e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmcmanus
bugs1024015
milestone33.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1024015 - Only enable HTTP response timeout when TCP Keepalives are disabled for HTTP r=mcmanus
modules/libpref/src/init/all.js
netwerk/protocol/http/nsHttpConnection.cpp
netwerk/protocol/http/nsHttpHandler.cpp
netwerk/protocol/http/nsHttpHandler.h
netwerk/test/unit/test_httpResponseTimeout.js
--- a/modules/libpref/src/init/all.js
+++ b/modules/libpref/src/init/all.js
@@ -983,17 +983,17 @@ pref("network.http.default-socket-type",
 
 // There is a problem with some IIS7 servers that don't close the connection
 // properly after it times out (bug #491541). Default timeout on IIS7 is
 // 120 seconds. We need to reuse or drop the connection within this time.
 // We set the timeout a little shorter to keep a reserve for cases when
 // the packet is lost or delayed on the route.
 pref("network.http.keep-alive.timeout", 115);
 
-// Timeout connections if an initial response is not received after 10 mins.
+// Timeout connections if an initial response is not received after 5 mins.
 pref("network.http.response.timeout", 300);
 
 // Limit the absolute number of http connections.
 // Note: the socket transport service will clamp the number below 256 if the OS
 // cannot allocate that many FDs, and it also always tries to reserve up to 250
 // file descriptors for things other than sockets.
 pref("network.http.max-connections", 256);
 
--- a/netwerk/protocol/http/nsHttpConnection.cpp
+++ b/netwerk/protocol/http/nsHttpConnection.cpp
@@ -412,17 +412,18 @@ nsHttpConnection::Activate(nsAHttpTransa
     }
 
     // Clear the per activation counter
     mCurrentBytesRead = 0;
 
     // The overflow state is not needed between activations
     mInputOverflow = nullptr;
 
-    mResponseTimeoutEnabled = mTransaction->ResponseTimeout() > 0 &&
+    mResponseTimeoutEnabled = gHttpHandler->ResponseTimeoutEnabled() &&
+                              mTransaction->ResponseTimeout() > 0 &&
                               mTransaction->ResponseTimeoutEnabled();
 
     rv = StartShortLivedTCPKeepalives();
     if (NS_FAILED(rv)) {
         LOG(("nsHttpConnection::Activate [%p] "
              "StartShortLivedTCPKeepalives failed rv[0x%x]",
              this, rv));
     }
@@ -1105,16 +1106,19 @@ nsHttpConnection::ReadTimeoutTick(PRInte
     // Spdy implements some timeout handling using the SPDY ping frame.
     if (mSpdySession) {
         return mSpdySession->ReadTimeoutTick(now);
     }
 
     uint32_t nextTickAfter = UINT32_MAX;
     // Timeout if the response is taking too long to arrive.
     if (mResponseTimeoutEnabled) {
+        NS_WARN_IF_FALSE(gHttpHandler->ResponseTimeoutEnabled(),
+                         "Timing out a response, but response timeout is disabled!");
+
         PRIntervalTime initialResponseDelta = now - mLastWriteTime;
 
         if (initialResponseDelta > mTransaction->ResponseTimeout()) {
             LOG(("canceling transaction: no response for %ums: timeout is %dms\n",
                  PR_IntervalToMilliseconds(initialResponseDelta),
                  PR_IntervalToMilliseconds(mTransaction->ResponseTimeout())));
 
             mResponseTimeoutEnabled = false;
--- a/netwerk/protocol/http/nsHttpHandler.cpp
+++ b/netwerk/protocol/http/nsHttpHandler.cpp
@@ -137,17 +137,18 @@ nsHttpHandler::nsHttpHandler()
     , mReferrerLevel(0xff) // by default we always send a referrer
     , mSpoofReferrerSource(false)
     , mReferrerTrimmingPolicy(0)
     , mReferrerXOriginPolicy(0)
     , mFastFallbackToIPv4(false)
     , mProxyPipelining(true)
     , mIdleTimeout(PR_SecondsToInterval(10))
     , mSpdyTimeout(PR_SecondsToInterval(180))
-    , mResponseTimeout(PR_SecondsToInterval(600))
+    , mResponseTimeout(PR_SecondsToInterval(300))
+    , mResponseTimeoutEnabled(false)
     , mMaxRequestAttempts(10)
     , mMaxRequestDelay(10)
     , mIdleSynTimeout(250)
     , mPipeliningEnabled(false)
     , mMaxConnections(24)
     , mMaxPersistentConnectionsPerServer(2)
     , mMaxPersistentConnectionsPerProxy(4)
     , mMaxPipelinedRequests(32)
@@ -1450,16 +1451,20 @@ nsHttpHandler::PrefsChanged(nsIPrefBranc
     if (PREF_CHANGED(HTTP_PREF("tcp_keepalive.long_lived_idle_time"))) {
         rv = prefs->GetIntPref(
             HTTP_PREF("tcp_keepalive.long_lived_idle_time"), &val);
         if (NS_SUCCEEDED(rv) && val > 0)
             mTCPKeepaliveLongLivedIdleTimeS = clamped(val,
                                                       1, kMaxTCPKeepIdle);
     }
 
+    // Enable HTTP response timeout if TCP Keepalives are disabled.
+    mResponseTimeoutEnabled = !mTCPKeepaliveShortLivedEnabled &&
+                              !mTCPKeepaliveLongLivedEnabled;
+
 #undef PREF_CHANGED
 #undef MULTI_PREF_CHANGED
 }
 
 
 /**
  * Static method called by mPipelineTestTimer when it expires.
  */
--- a/netwerk/protocol/http/nsHttpHandler.h
+++ b/netwerk/protocol/http/nsHttpHandler.h
@@ -71,17 +71,20 @@ public:
     uint8_t        ReferrerLevel()           { return mReferrerLevel; }
     bool           SpoofReferrerSource()     { return mSpoofReferrerSource; }
     uint8_t        ReferrerTrimmingPolicy()  { return mReferrerTrimmingPolicy; }
     uint8_t        ReferrerXOriginPolicy()   { return mReferrerXOriginPolicy; }
     bool           SendSecureXSiteReferrer() { return mSendSecureXSiteReferrer; }
     uint8_t        RedirectionLimit()        { return mRedirectionLimit; }
     PRIntervalTime IdleTimeout()             { return mIdleTimeout; }
     PRIntervalTime SpdyTimeout()             { return mSpdyTimeout; }
-    PRIntervalTime ResponseTimeout()         { return mResponseTimeout; }
+    PRIntervalTime ResponseTimeout() {
+      return mResponseTimeoutEnabled ? mResponseTimeout : 0;
+    }
+    PRIntervalTime ResponseTimeoutEnabled()  { return mResponseTimeoutEnabled; }
     uint16_t       MaxRequestAttempts()      { return mMaxRequestAttempts; }
     const char    *DefaultSocketType()       { return mDefaultSocketType.get(); /* ok to return null */ }
     uint32_t       PhishyUserPassLength()    { return mPhishyUserPassLength; }
     uint8_t        GetQoSBits()              { return mQoSBits; }
     uint16_t       GetIdleSynTimeout()       { return mIdleSynTimeout; }
     bool           FastFallbackToIPv4()      { return mFastFallbackToIPv4; }
     bool           ProxyPipelining()         { return mProxyPipelining; }
     uint32_t       MaxSocketCount();
@@ -357,16 +360,17 @@ private:
     uint8_t  mReferrerTrimmingPolicy;
     uint8_t  mReferrerXOriginPolicy;
 
     bool mFastFallbackToIPv4;
     bool mProxyPipelining;
     PRIntervalTime mIdleTimeout;
     PRIntervalTime mSpdyTimeout;
     PRIntervalTime mResponseTimeout;
+    bool mResponseTimeoutEnabled;
 
     uint16_t mMaxRequestAttempts;
     uint16_t mMaxRequestDelay;
     uint16_t mIdleSynTimeout;
 
     bool     mPipeliningEnabled;
     uint16_t mMaxConnections;
     uint8_t  mMaxPersistentConnectionsPerServer;
--- a/netwerk/test/unit/test_httpResponseTimeout.js
+++ b/netwerk/test/unit/test_httpResponseTimeout.js
@@ -1,89 +1,163 @@
+/* -*- Mode: javascript; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* vim: set ts=8 sts=2 et sw=2 tw=80: */
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+"use strict";
+
 Cu.import("resource://testing-common/httpd.js");
 
 var baseURL;
 const kResponseTimeoutPref = "network.http.response.timeout";
 const kResponseTimeout = 1;
+const kShortLivedKeepalivePref =
+  "network.http.tcp_keepalive.short_lived_connections";
+const kLongLivedKeepalivePref =
+  "network.http.tcp_keepalive.long_lived_connections";
 
 const prefService = Cc["@mozilla.org/preferences-service;1"]
                        .getService(Ci.nsIPrefBranch);
 
 var server = new HttpServer();
 
-function TimeoutListener(enableTimeout, nextTest) {
-  this.enableTimeout = enableTimeout;
-  this.nextTest = nextTest;
+function TimeoutListener(expectResponse) {
+  this.expectResponse = expectResponse;
 }
 
 TimeoutListener.prototype = {
   onStartRequest: function (request, ctx) {
   },
 
   onDataAvailable: function (request, ctx, stream) {
   },
 
   onStopRequest: function (request, ctx, status) {
-    if (this.enableTimeout) {
+    if (this.expectResponse) {
+      do_check_eq(status, Cr.NS_OK);
+    } else {
       do_check_eq(status, Cr.NS_ERROR_NET_TIMEOUT);
-    } else {
-      do_check_eq(status, Cr.NS_OK);
     }
 
-    if (this.nextTest) {
-      this.nextTest();
-    } else {
-      server.stop(serverStopListener);
-    }
+    run_next_test();
   },
 };
 
 function serverStopListener() {
   do_test_finished();
 }
 
-function testTimeout(timeoutEnabled, nextTest) {
+function testTimeout(timeoutEnabled, expectResponse) {
   // Set timeout pref.
   if (timeoutEnabled) {
     prefService.setIntPref(kResponseTimeoutPref, kResponseTimeout);
   } else {
     prefService.setIntPref(kResponseTimeoutPref, 0);
   }
 
   var ios = Cc["@mozilla.org/network/io-service;1"]
   .getService(Ci.nsIIOService);
   var chan = ios.newChannel(baseURL, null, null)
   .QueryInterface(Ci.nsIHttpChannel);
-  var listener = new TimeoutListener(timeoutEnabled, nextTest);
+  var listener = new TimeoutListener(expectResponse);
   chan.asyncOpen(listener, null);
 }
 
 function testTimeoutEnabled() {
-  testTimeout(true, testTimeoutDisabled);
+  // Set a timeout value; expect a timeout and no response.
+  testTimeout(true, false);
 }
 
 function testTimeoutDisabled() {
-  testTimeout(false, null);
+  // Set a timeout value of 0; expect a response.
+  testTimeout(false, true);
+}
+
+function testTimeoutDisabledByShortLivedKeepalives() {
+  // Enable TCP Keepalives for short lived HTTP connections.
+  prefService.setBoolPref(kShortLivedKeepalivePref, true);
+  prefService.setBoolPref(kLongLivedKeepalivePref, false);
+
+  // Try to set a timeout value, but expect a response without timeout.
+  testTimeout(true, true);
+}
+
+function testTimeoutDisabledByLongLivedKeepalives() {
+  // Enable TCP Keepalives for long lived HTTP connections.
+  prefService.setBoolPref(kShortLivedKeepalivePref, false);
+  prefService.setBoolPref(kLongLivedKeepalivePref, true);
+
+  // Try to set a timeout value, but expect a response without timeout.
+  testTimeout(true, true);
+}
+
+function testTimeoutDisabledByBothKeepalives() {
+  // Enable TCP Keepalives for short and long lived HTTP connections.
+  prefService.setBoolPref(kShortLivedKeepalivePref, true);
+  prefService.setBoolPref(kLongLivedKeepalivePref, true);
+
+  // Try to set a timeout value, but expect a response without timeout.
+  testTimeout(true, true);
 }
 
-function run_test() {
-  // Start server; will be stopped after second test.
+function setup_tests() {
+  // Start tests with timeout enabled, i.e. disable TCP keepalives for HTTP.
+  // Reset pref in cleanup.
+  if (prefService.getBoolPref(kShortLivedKeepalivePref)) {
+    prefService.setBoolPref(kShortLivedKeepalivePref, false);
+    do_register_cleanup(function() {
+      prefService.setBoolPref(kShortLivedKeepalivePref, true);
+    });
+  }
+  if (prefService.getBoolPref(kLongLivedKeepalivePref)) {
+    prefService.setBoolPref(kLongLivedKeepalivePref, false);
+    do_register_cleanup(function() {
+      prefService.setBoolPref(kLongLivedKeepalivePref, true);
+    });
+  }
+
+  var tests = [
+    // Enable with a timeout value >0;
+    testTimeoutEnabled,
+    // Disable with a timeout value of 0;
+    testTimeoutDisabled,
+    // Disable by enabling TCP keepalive for short-lived HTTP connections.
+    testTimeoutDisabledByShortLivedKeepalives,
+    // Disable by enabling TCP keepalive for long-lived HTTP connections.
+    testTimeoutDisabledByLongLivedKeepalives,
+    // Disable by enabling TCP keepalive for both HTTP connection types.
+    testTimeoutDisabledByBothKeepalives
+  ];
+
+  for (var i=0; i < tests.length; i++) {
+    add_test(tests[i]);
+  }
+}
+
+function setup_http_server() {
+  // Start server; will be stopped at test cleanup time.
   server.start(-1);
   baseURL = "http://localhost:" + server.identity.primaryPort + "/";
   do_print("Using baseURL: " + baseURL);
   server.registerPathHandler('/', function(metadata, response) {
     // Wait until the timeout should have passed, then respond.
     response.processAsync();
 
     do_timeout((kResponseTimeout+1)*1000 /* ms */, function() {
       response.setStatusLine(metadata.httpVersion, 200, "OK");
       response.write("Hello world");
       response.finish();
     });
   });
-
-  // First test checks timeout enabled.
-  // Second test (in callback) checks timeout disabled.
-  testTimeoutEnabled();
-
-  do_test_pending();
+  do_register_cleanup(function() {
+    server.stop(serverStopListener);
+  });
 }
 
+function run_test() {
+  setup_http_server();
+
+  setup_tests();
+
+  run_next_test();
+}