When an idle socket is added back to a socket pool, check for stalled jobs in lower pools

net/spdy/spdy_session.cc

Index: net/spdy/spdy_session.cc
===================================================================
--- net/spdy/spdy_session.cc	(revision 218288)
+++ net/spdy/spdy_session.cc	(working copy)
@@ -517,7 +517,7 @@
     error = OK;
   if (error == OK) {
     DCHECK_NE(availability_state_, STATE_CLOSED);
-    connection_->AddLayeredPool(this);
+    connection_->AddHigherLayeredPool(this);
     if (enable_sending_initial_data_)
       SendInitialData();
     pool_ = pool;
@@ -1050,7 +1050,20 @@
   if (owned_stream->type() == SPDY_PUSH_STREAM)
       unclaimed_pushed_streams_.erase(owned_stream->url());

[akalin, 2013/08/21 17:29:03]

can you fix this indent while you're here?

[mmenke, 2013/08/21 18:17:16]

Done.

 
+  // If there are no active streams, and the socket pool is stalled, try to free
+  // up a socket slot.  This must be done before the call to DeleteStream, as it
+  // may delete the session.
+  if (active_streams_.empty() && availability_state_ == STATE_AVAILABLE &&

[akalin, 2013/08/21 17:29:03]

i think this should go after DeleteStream, as otherwise the stream's delegate
won't get called in some cases (if we call CloseIfIdle synchronously -- see
below).

You can do the same trick that DeleteStream() does, and also check if it's
already closed:

WeakPtr<SpdySession> weak_this = GetWeakPtr();

DeleteStream(...)

if (!weak_this)
  return;

if (availability_state_ == STATE_CLOSED)
  return;

if (active_streams_.empty() && connection_->IsPoolStalled())
  ...

[mmenke, 2013/08/21 18:17:16]

Done.

+      connection_->IsPoolStalled()) {
+    // Can't close the session synchronously, because callers don't expect it.
+    base::MessageLoop::current()->PostTask(

[akalin, 2013/08/21 17:29:03]

you should be able to just call CloseIfIdle here directly -- callers should
definitely expect that closing an active stream can close the session, too.

[mmenke, 2013/08/21 18:17:16]

Done.

When I wrote this CL a month ago, that resulted in crashing.  When a stream was
shut down (cancelled?), it would delete the stream, and then write the stream
closed message to the socket.  Deleting the session here in that case would
cause a crash.  This no longer seems to be the case.

+        FROM_HERE, base::Bind(&SpdySession::CloseIfIdle,
+                              weak_factory_.GetWeakPtr()));
+  }
+
   DeleteStream(owned_stream.Pass(), status);
+
+  // |this| may have been deleted at this point.
 }
 
 void SpdySession::CloseCreatedStreamIterator(CreatedStreamSet::iterator it,
@@ -2925,4 +2938,11 @@
   return 0;
 }
 
+void SpdySession::CloseIfIdle() {
+  // If the session isn't idle, or it has already been closed, do nothing.
+  if (!active_streams_.empty() || availability_state_ == STATE_CLOSED || !pool_)
+    return;
+  DoCloseSession(ERR_CONNECTION_CLOSED, "Closing idle connection.");
+}
+
 }  // namespace net