Close idle connections / SPDY sessions when needed

net/socket/client_socket_pool.cc

Index: net/socket/client_socket_pool.cc
diff --git a/net/socket/client_socket_pool.cc b/net/socket/client_socket_pool.cc
index a1967efdbab2b12a340ad7cb37e8af1596d82f45..871ff6165786f5b4b06e8fa8089c0a918158efb3 100644
--- a/net/socket/client_socket_pool.cc
+++ b/net/socket/client_socket_pool.cc
@@ -21,6 +21,24 @@ int g_used_idle_socket_timeout_s = 300;  // 5 minutes
 
 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);