Close idle connections / SPDY sessions when needed

net/socket/client_socket_pool.cc

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/socket/client_socket_pool.h"
 
 #include "base/logging.h"
 
 namespace {
 
 // The maximum duration, in seconds, to keep unused idle persistent sockets
 // alive.
 // TODO(ziadh): Change this timeout after getting histogram data on how long it
 // should be.
 int g_unused_idle_socket_timeout_s = 10;
 
 // The maximum duration, in seconds, to keep used idle persistent sockets alive.
 int g_used_idle_socket_timeout_s = 300;  // 5 minutes
 
 }  // namespace
 
 namespace net {
 
+LayeredPool::LayeredPool() {
+  magic_value_ = 0xDEADBEEF;
+}
+
+LayeredPool::~LayeredPool() {
+  magic_value_ = 0;

[eroman, 2012/03/28 18:02:50]

Two comments:
  (1) Define the magic values as an enum. This will simplify things for you/me
when looking at minidumps, since the debugger can resolve to the symbolic name.
For instance:

enum MagicValue {
  ALIVE = 0xCA11AB1E,
  DEAD = 0xDEADBEEF;
};

(2) You should absolutely not use 0 for your magic value on deletion. Zero tends
to be far too common of a value, which means there are more ways for this slot
to become zero than just destruction. Something with more bits of randomness
(like 0xDEADBEFF) would be preferable.

[Ryan Hamilton, 2012/03/28 18:15:44]

Done!
Good point!  I wondered about this but since I thought we were merely checking
alive vs. !alive 0 would be sufficient.  But i like your approach which can help
us differentiate between a freed object and reused memory.

+  stack_trace_ = base::debug::StackTrace();
+}
+
+void crash(base::debug::StackTrace trace) {
+  CHECK(false);
+}
+
+void LayeredPool::CrashIfFreed() {
+ if (magic_value_ != 0xDEADBEEF)
+   crash(stack_trace_);

[eroman, 2012/03/28 18:02:50]

OK, so you will want to do some extra work here. Structure it like this:


if (magic_value_ != ALIVE) {
  MagicValue magic_value = magic_value_;
  base::debug::StackTrace deletion_callstack = stack_trace_;

  base::debug::Alias(&magic_value);
  base::debug::Alias(&deletion_callstack);

  CHECK(false);
}

We must explicitly copy the data we want onto the stack, otherwise there is no
guarantee that the minidump's memory will include it (since it is going to be in
the heap, somewhere off of the |this| pointer, which may not be recoverable).

Simply copying into local variables is insufficient since the compiler will
optimize it away. That is where the base::debug::Alias() comes it. It makes the
variable non-eliminatable by virtue of taking its address.

[Ryan Hamilton, 2012/03/28 18:15:44]

AH!  Awesome.  I kinda thought that adding the StackTrace as an argument to 
method might not be sufficient.  Thanks!

+}
+
 // static
 base::TimeDelta ClientSocketPool::unused_idle_socket_timeout() {
   return base::TimeDelta::FromSeconds(g_unused_idle_socket_timeout_s);
 }
 
 // static
 void ClientSocketPool::set_unused_idle_socket_timeout(base::TimeDelta timeout) {
   DCHECK_GT(timeout.InSeconds(), 0);
   g_unused_idle_socket_timeout_s = timeout.InSeconds();
 }
 
 // static
 base::TimeDelta ClientSocketPool::used_idle_socket_timeout() {
   return base::TimeDelta::FromSeconds(g_used_idle_socket_timeout_s);
 }
 
 // static
 void ClientSocketPool::set_used_idle_socket_timeout(base::TimeDelta timeout) {
   DCHECK_GT(timeout.InSeconds(), 0);
   g_used_idle_socket_timeout_s = timeout.InSeconds();
 }
 
 ClientSocketPool::ClientSocketPool() {}
 
 ClientSocketPool::~ClientSocketPool() {}
 
 }  // namespace net