Bug 1024015 - Only enable HTTP response timeout when TCP Keepalives are disabled for HTTP. r=mcmanus, a=sledru
authorSteve Workman <sworkman@mozilla.com>
Tue, 01 Jul 2014 13:46:00 -0400
changeset 207576 e170f06d541f15329cbc63cd492403be8ec1a8ea
parent 207575 874c3fc4f6aeff61a934bb319c198de273b2c492
child 207577 82178eb6f403e5704fa3f8dc32684114b57430d7
push id3741
push userasasaki@mozilla.com
push dateMon, 21 Jul 2014 20:25:18 +0000
treeherdermozilla-beta@4d6f46f5af68 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmcmanus, sledru
bugs1024015
milestone32.0a2
Bug 1024015 - Only enable HTTP response timeout when TCP Keepalives are disabled for HTTP. r=mcmanus, a=sledru
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
@@ -973,17 +973,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(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
@@ -72,17 +72,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();
+}