New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 92244 link

Starred by 30 users

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2013
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

Use of different socket pool instances of SSL and non-SSL results in inability to close idle sockets for different types

Project Member Reported by willchan@chromium.org, Aug 9 2011

Issue description

We have a core socket pool implementation class: ClientSocketPoolBaseHelper. Each socket pool instance embeds this object. This object implements the policy that, if we have hit the max number of sockets in the pool and receive a request for a new socket that we don't already have available, then we close an idle socket to free up a slot to connect a new socket.

Note that we use a socket pool layering scheme. For proxies, we have a TransportClientSocketPool that manages the TCP connections directly to the proxy. On top of that, we may have a SSLClientSocketPool for when we do SSL tunneling over the proxy connection via HTTP CONNECT. Note that when we have an idle SSL socket, it sits idle in the SSLClientSocketPool, but it looks like it's non-idle ("active") to the TransportClientSocketPool whose socket the SSL connection is being tunneled over.

Therefore, when we've maxed out on the 32 connections in the TransportClientSocketPool for the proxy, then if we try to open a new non-SSL socket, we can only close existing idle non-SSL sockets, not idle SSL sockets. Likewise, when we try to open new SSL sockets, we can only close existing idle SSL sockets, not idle non-SSL sockets. This can lead to problems when users have lots of https websites open, and can be further exacerbated by preconnecting SSL sockets for SPDY ( issue 66472 ) and not closing idle SPDY sessions ( issue 62364 ). But the root cause is this separation of idle socket connection management across different socket pool instances.
 
Cc: cbentzel@chromium.org
Labels: Mstone-17
Owner: willchan@chromium.org
Status: Assigned
Cc: mmenke@chromium.org willchan@chromium.org
Owner: rtenneti@chromium.org
I think Raman's on this. Maybe Matt if he's interested too. I believe the fix is a nontrivial design change though.
I'd be happy to help with this, when I have time.
Cc: -willchan@chromium.org rtenneti@chromium.org
Owner: willchan@chromium.org
Status: Started
I've got a changlelist most of the way through review which will fix this.
Project Member

Comment 5 by bugdroid1@chromium.org, Nov 30 2011

The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=112130

------------------------------------------------------------------------
r112130 | willchan@chromium.org | Tue Nov 29 20:57:26 PST 2011

Changed paths:
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_pool_base_unittest.cc?r1=112130&r2=112129&pathrev=112130
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_pool_base.h?r1=112130&r2=112129&pathrev=112130
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_pool.h?r1=112130&r2=112129&pathrev=112130
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_pool_base.cc?r1=112130&r2=112129&pathrev=112130
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/spdy/spdy_session.cc?r1=112130&r2=112129&pathrev=112130
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/http/http_network_transaction_unittest.cc?r1=112130&r2=112129&pathrev=112130
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/transport_client_socket_pool.cc?r1=112130&r2=112129&pathrev=112130
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_handle.h?r1=112130&r2=112129&pathrev=112130
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/http/http_proxy_client_socket_pool.cc?r1=112130&r2=112129&pathrev=112130
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/ssl_client_socket_pool.cc?r1=112130&r2=112129&pathrev=112130
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/socks_client_socket_pool.h?r1=112130&r2=112129&pathrev=112130
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/socks_client_socket_pool.cc?r1=112130&r2=112129&pathrev=112130
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/http/http_proxy_client_socket_pool.h?r1=112130&r2=112129&pathrev=112130
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/ssl_client_socket_pool.h?r1=112130&r2=112129&pathrev=112130
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/spdy/spdy_session.h?r1=112130&r2=112129&pathrev=112130
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/transport_client_socket_pool.h?r1=112130&r2=112129&pathrev=112130
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_handle.cc?r1=112130&r2=112129&pathrev=112130

Close idle connections / SPDY sessions when needed.

Due to the idle connection state being held by different socket pools, it's possible for one socket pool to hold an idle socket in a lower layer socket pool. From the lower level socket pool's perspective, the socket is being "actively" used. From the higher socket pool's (including SpdySession, which is more of a connection manager) perspective, the connection is idle and can be closed if we have hit a limit.

Normally this isn't a big deal, except when we have a lot of idle SPDY connections and are connecting via a proxy, so we have low connection limits through the proxy server. We address this problem by allowing lower-level socket pools to tell higher level socket pools to close a socket. 

BUG= 62364 , 92244 
TEST=none


Review URL: http://codereview.chromium.org/8340012
------------------------------------------------------------------------
Project Member

Comment 6 by bugdroid1@chromium.org, Nov 30 2011

The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=112134

------------------------------------------------------------------------
r112134 | sail@chromium.org | Tue Nov 29 22:14:42 PST 2011

Changed paths:
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_pool_base_unittest.cc?r1=112134&r2=112133&pathrev=112134
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_pool_base.h?r1=112134&r2=112133&pathrev=112134
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_pool.h?r1=112134&r2=112133&pathrev=112134
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_pool_base.cc?r1=112134&r2=112133&pathrev=112134
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/spdy/spdy_session.cc?r1=112134&r2=112133&pathrev=112134
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/http/http_network_transaction_unittest.cc?r1=112134&r2=112133&pathrev=112134
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/transport_client_socket_pool.cc?r1=112134&r2=112133&pathrev=112134
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_handle.h?r1=112134&r2=112133&pathrev=112134
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/http/http_proxy_client_socket_pool.cc?r1=112134&r2=112133&pathrev=112134
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/ssl_client_socket_pool.cc?r1=112134&r2=112133&pathrev=112134
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/socks_client_socket_pool.h?r1=112134&r2=112133&pathrev=112134
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/socks_client_socket_pool.cc?r1=112134&r2=112133&pathrev=112134
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/http/http_proxy_client_socket_pool.h?r1=112134&r2=112133&pathrev=112134
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/ssl_client_socket_pool.h?r1=112134&r2=112133&pathrev=112134
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/spdy/spdy_session.h?r1=112134&r2=112133&pathrev=112134
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/transport_client_socket_pool.h?r1=112134&r2=112133&pathrev=112134
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_handle.cc?r1=112134&r2=112133&pathrev=112134

Revert 112130 - Close idle connections / SPDY sessions when needed.

Due to the idle connection state being held by different socket pools, it's possible for one socket pool to hold an idle socket in a lower layer socket pool. From the lower level socket pool's perspective, the socket is being "actively" used. From the higher socket pool's (including SpdySession, which is more of a connection manager) perspective, the connection is idle and can be closed if we have hit a limit.

Normally this isn't a big deal, except when we have a lot of idle SPDY connections and are connecting via a proxy, so we have low connection limits through the proxy server. We address this problem by allowing lower-level socket pools to tell higher level socket pools to close a socket. 

BUG= 62364 , 92244 ,  105839 
TEST=none


Review URL: http://codereview.chromium.org/8340012

TBR=willchan@chromium.org
Review URL: http://codereview.chromium.org/8745007
------------------------------------------------------------------------
Project Member

Comment 7 by bugdroid1@chromium.org, Dec 6 2011

The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=113300

------------------------------------------------------------------------
r113300 | rtenneti@chromium.org | Tue Dec 06 15:53:21 PST 2011

Changed paths:
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_pool_base_unittest.cc?r1=113300&r2=113299&pathrev=113300
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_pool_base.h?r1=113300&r2=113299&pathrev=113300
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_pool.h?r1=113300&r2=113299&pathrev=113300
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_pool_base.cc?r1=113300&r2=113299&pathrev=113300
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/spdy/spdy_session.cc?r1=113300&r2=113299&pathrev=113300
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/http/http_network_transaction_unittest.cc?r1=113300&r2=113299&pathrev=113300
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/transport_client_socket_pool.cc?r1=113300&r2=113299&pathrev=113300
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_handle.h?r1=113300&r2=113299&pathrev=113300
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/http/http_proxy_client_socket_pool.cc?r1=113300&r2=113299&pathrev=113300
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/ssl_client_socket_pool.cc?r1=113300&r2=113299&pathrev=113300
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/socks_client_socket_pool.h?r1=113300&r2=113299&pathrev=113300
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/socks_client_socket_pool.cc?r1=113300&r2=113299&pathrev=113300
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/http/http_proxy_client_socket_pool.h?r1=113300&r2=113299&pathrev=113300
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/ssl_client_socket_pool.h?r1=113300&r2=113299&pathrev=113300
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/spdy/spdy_session.h?r1=113300&r2=113299&pathrev=113300
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/transport_client_socket_pool.h?r1=113300&r2=113299&pathrev=113300
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_handle.cc?r1=113300&r2=113299&pathrev=113300

Revert of 112134 of Revert 112130 - Close idle connections / SPDY sessions when needed.

Due to the idle connection state being held by different socket pools, it's possible for one socket pool to hold an idle socket in a lower layer socket pool. From the lower level socket pool's perspective, the socket is being "actively" used. From the higher socket pool's (including SpdySession, which is more of a connection manager) perspective, the connection is idle and can be closed if we have hit a limit.

Normally this isn't a big deal, except when we have a lot of idle SPDY connections and are connecting via a proxy, so we have low connection limits through the proxy server. We address this problem by allowing lower-level socket pools to tell higher level socket pools to close a socket. 

Fixed ASAN test failures by removing  .Times(1) and .Times(2)  from CloseMultipleIdleSocketsHeldByLayeredPoolWhenNeeded unittest (this removes the tests relying on the order of std::set in CloseOneIdleConnectionInLayeredPool). ASAN is probably causing the memory allocator to allocate the pools differently. The std::set is ordered by LayeredPool* which is the address of the LayeredPool (willchan).


BUG= 62364 , 92244 ,  105839 
TEST=none


Review URL: http://codereview.chromium.org/8340012

TBR=willchan@chromium.org
Review URL: http://codereview.chromium.org/8745007

TBR=willchan@chromium.org

Review URL: http://codereview.chromium.org/8803019
------------------------------------------------------------------------
Cc: kerz@chromium.org
@kerz: I realize we missed the cutoff for Mstone-17. I'd like to request consideration for backporting to Mstone-17. I'd like to let the changelist stabilize on trunk for a week or so, and then if it doesn't blow up, merge it to Mstone-17. The reason is this bug can lead to browser connections hanging completely in situations where lots of SSL connections are open and we are using proxies, which renders the browser completely useless. It's an edge case, yet definitely one which we've seen users hit a number of times, so I'd like to get it in if possible.
Project Member

Comment 9 by bugdroid1@chromium.org, Dec 7 2011

The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=113305

------------------------------------------------------------------------
r113305 | rtenneti@chromium.org | Tue Dec 06 16:40:01 PST 2011

Changed paths:
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_pool_base_unittest.cc?r1=113305&r2=113304&pathrev=113305
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_pool_base.h?r1=113305&r2=113304&pathrev=113305
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_pool.h?r1=113305&r2=113304&pathrev=113305
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_pool_base.cc?r1=113305&r2=113304&pathrev=113305
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/spdy/spdy_session.cc?r1=113305&r2=113304&pathrev=113305
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/http/http_network_transaction_unittest.cc?r1=113305&r2=113304&pathrev=113305
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/transport_client_socket_pool.cc?r1=113305&r2=113304&pathrev=113305
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_handle.h?r1=113305&r2=113304&pathrev=113305
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/http/http_proxy_client_socket_pool.cc?r1=113305&r2=113304&pathrev=113305
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/ssl_client_socket_pool.cc?r1=113305&r2=113304&pathrev=113305
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/socks_client_socket_pool.h?r1=113305&r2=113304&pathrev=113305
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/socks_client_socket_pool.cc?r1=113305&r2=113304&pathrev=113305
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/http/http_proxy_client_socket_pool.h?r1=113305&r2=113304&pathrev=113305
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/ssl_client_socket_pool.h?r1=113305&r2=113304&pathrev=113305
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/spdy/spdy_session.h?r1=113305&r2=113304&pathrev=113305
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/transport_client_socket_pool.h?r1=113305&r2=113304&pathrev=113305
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_handle.cc?r1=113305&r2=113304&pathrev=113305

Revert 113300 - Revert of 112134 of Revert 112130 - Close idle connections / SPDY sessions when needed.

Due to the idle connection state being held by different socket pools, it's possible for one socket pool to hold an idle socket in a lower layer socket pool. From the lower level socket pool's perspective, the socket is being "actively" used. From the higher socket pool's (including SpdySession, which is more of a connection manager) perspective, the connection is idle and can be closed if we have hit a limit.

Normally this isn't a big deal, except when we have a lot of idle SPDY connections and are connecting via a proxy, so we have low connection limits through the proxy server. We address this problem by allowing lower-level socket pools to tell higher level socket pools to close a socket. 

Fixed ASAN test failures by removing  .Times(1) and .Times(2)  from CloseMultipleIdleSocketsHeldByLayeredPoolWhenNeeded unittest (this removes the tests relying on the order of std::set in CloseOneIdleConnectionInLayeredPool). ASAN is probably causing the memory allocator to allocate the pools differently. The std::set is ordered by LayeredPool* which is the address of the LayeredPool (willchan).


BUG= 62364 , 92244 ,  105839 
TEST=none


Review URL: http://codereview.chromium.org/8340012

TBR=willchan@chromium.org
Review URL: http://codereview.chromium.org/8745007

TBR=willchan@chromium.org

Review URL: http://codereview.chromium.org/8803019

TBR=rtenneti@chromium.org
Review URL: http://codereview.chromium.org/8825014
------------------------------------------------------------------------

Comment 10 by kerz@chromium.org, Dec 7 2011

Let's talk after it's baked.
I don't think it's an "edge" issue, as you can see from  issue #44501 , just opening some google apps at startup makes the problem appear (with prediction enabled).

And you can see many more people hit by this on  issue #55046 .

The only reason not everyone experiences this bug is that not everyone uses a proxy, but all of us that do are hit very hard.
Project Member

Comment 12 by bugdroid1@chromium.org, Dec 7 2011

The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=113405

------------------------------------------------------------------------
r113405 | rtenneti@google.com | Wed Dec 07 08:56:59 PST 2011

Changed paths:
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_pool_base_unittest.cc?r1=113405&r2=113404&pathrev=113405
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_pool_base.h?r1=113405&r2=113404&pathrev=113405
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_pool.h?r1=113405&r2=113404&pathrev=113405
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_pool_base.cc?r1=113405&r2=113404&pathrev=113405
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/spdy/spdy_session.cc?r1=113405&r2=113404&pathrev=113405
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/http/http_network_transaction_unittest.cc?r1=113405&r2=113404&pathrev=113405
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/transport_client_socket_pool.cc?r1=113405&r2=113404&pathrev=113405
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_handle.h?r1=113405&r2=113404&pathrev=113405
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/http/http_proxy_client_socket_pool.cc?r1=113405&r2=113404&pathrev=113405
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/ssl_client_socket_pool.cc?r1=113405&r2=113404&pathrev=113405
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/socks_client_socket_pool.h?r1=113405&r2=113404&pathrev=113405
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/socks_client_socket_pool.cc?r1=113405&r2=113404&pathrev=113405
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/http/http_proxy_client_socket_pool.h?r1=113405&r2=113404&pathrev=113405
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/ssl_client_socket_pool.h?r1=113405&r2=113404&pathrev=113405
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/spdy/spdy_session.h?r1=113405&r2=113404&pathrev=113405
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/transport_client_socket_pool.h?r1=113405&r2=113404&pathrev=113405
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_handle.cc?r1=113405&r2=113404&pathrev=113405

Revert 113305 - Revert 113300 - Revert of 112134 of Revert 112130 - Close idle connections / SPDY sessions when needed.

Due to the idle connection state being held by different socket pools, it's possible for one socket pool to hold an idle socket in a lower layer socket pool. From the lower level socket pool's perspective, the socket is being "actively" used. From the higher socket pool's (including SpdySession, which is more of a connection manager) perspective, the connection is idle and can be closed if we have hit a limit.

Normally this isn't a big deal, except when we have a lot of idle SPDY connections and are connecting via a proxy, so we have low connection limits through the proxy server. We address this problem by allowing lower-level socket pools to tell higher level socket pools to close a socket.

Fixed ASAN test failures by removing .Times(1) and .Times(2) from CloseMultipleIdleSocketsHeldByLayeredPoolWhenNeeded unittest (this removes the tests relying on the order of std::set in CloseOneIdleConnectionInLayeredPool). ASAN is prob
ably causing the memory allocator to allocate the pools differently. The std::set is ordered by LayeredPool* which is the address of the LayeredPool (willchan).

Added NET_EXPORT for layered_pool class defintion to fix windows shared compile.

BUG= 62364 ,  92244 ,  105839 
TEST=none


Review URL: http://codereview.chromium.org/8340012

TBR=willchan@chromium.org
Review URL: http://codereview.chromium.org/8745007

TBR=willchan@chromium.org

Review URL: http://codereview.chromium.org/8803019

TBR=rtenneti@chromium.org
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=113305

TBR=willchan@chromium.org
Review URL: http://codereview.chromium.org/8836002
------------------------------------------------------------------------

Comment 13 by k...@google.com, Dec 19 2011

Labels: -Mstone-17 Mstone-18 MovedFrom-17
Moving bugs marked as Started but not blockers from M17 to M18.  Please move back if you think this is a blocker, and add the ReleaseBlock-Stable label.  If you're able.
Project Member

Comment 14 by bugdroid1@chromium.org, Jan 24 2012

The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=118788

------------------------------------------------------------------------
r118788 | eroman@chromium.org | Mon Jan 23 18:49:33 PST 2012

Changed paths:
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_pool_base_unittest.cc?r1=118788&r2=118787&pathrev=118788
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_pool_base.h?r1=118788&r2=118787&pathrev=118788
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_pool.h?r1=118788&r2=118787&pathrev=118788
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_pool_base.cc?r1=118788&r2=118787&pathrev=118788
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/spdy/spdy_session.cc?r1=118788&r2=118787&pathrev=118788
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/http/http_network_transaction_unittest.cc?r1=118788&r2=118787&pathrev=118788
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/transport_client_socket_pool.cc?r1=118788&r2=118787&pathrev=118788
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_handle.h?r1=118788&r2=118787&pathrev=118788
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/http/http_proxy_client_socket_pool.cc?r1=118788&r2=118787&pathrev=118788
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/ssl_client_socket_pool.cc?r1=118788&r2=118787&pathrev=118788
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/socks_client_socket_pool.h?r1=118788&r2=118787&pathrev=118788
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/socks_client_socket_pool.cc?r1=118788&r2=118787&pathrev=118788
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/http/http_proxy_client_socket_pool.h?r1=118788&r2=118787&pathrev=118788
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/ssl_client_socket_pool.h?r1=118788&r2=118787&pathrev=118788
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/spdy/spdy_session.h?r1=118788&r2=118787&pathrev=118788
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/transport_client_socket_pool.h?r1=118788&r2=118787&pathrev=118788
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_handle.cc?r1=118788&r2=118787&pathrev=118788

Revert r113405, since it appears to be causing a crash and a hang. Also reverted r118506 since it is no longer applicable.

BUG= 109876 ,  110368 ,  62364 ,  92244 

Review URL: https://chromiumcodereview.appspot.com/9226039
------------------------------------------------------------------------

Comment 15 by kareng@google.com, Feb 7 2012

Labels: MovedFrom18 Mstone-19
Project Member

Comment 16 by bugdroid1@chromium.org, Mar 20 2012

The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=127717

------------------------------------------------------------------------
r127717 | rch@chromium.org | Tue Mar 20 10:37:28 PDT 2012

Changed paths:
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_pool_base_unittest.cc?r1=127717&r2=127716&pathrev=127717
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_pool_base.h?r1=127717&r2=127716&pathrev=127717
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_pool.h?r1=127717&r2=127716&pathrev=127717
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_pool_base.cc?r1=127717&r2=127716&pathrev=127717
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/spdy/spdy_session.cc?r1=127717&r2=127716&pathrev=127717
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/transport_client_socket_pool.cc?r1=127717&r2=127716&pathrev=127717
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_handle.h?r1=127717&r2=127716&pathrev=127717
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/http/http_network_transaction_spdy2_unittest.cc?r1=127717&r2=127716&pathrev=127717
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/http/http_network_transaction_spdy3_unittest.cc?r1=127717&r2=127716&pathrev=127717
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/http/http_proxy_client_socket_pool.cc?r1=127717&r2=127716&pathrev=127717
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/ssl_client_socket_pool.cc?r1=127717&r2=127716&pathrev=127717
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/socks_client_socket_pool.h?r1=127717&r2=127716&pathrev=127717
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/http/http_network_transaction_spdy21_unittest.cc?r1=127717&r2=127716&pathrev=127717
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/socks_client_socket_pool.cc?r1=127717&r2=127716&pathrev=127717
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/http/http_proxy_client_socket_pool.h?r1=127717&r2=127716&pathrev=127717
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/ssl_client_socket_pool.h?r1=127717&r2=127716&pathrev=127717
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/spdy/spdy_session.h?r1=127717&r2=127716&pathrev=127717
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/spdy/spdy_session_spdy2_unittest.cc?r1=127717&r2=127716&pathrev=127717
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/transport_client_socket_pool.h?r1=127717&r2=127716&pathrev=127717
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_handle.cc?r1=127717&r2=127716&pathrev=127717
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/spdy/spdy_session_spdy3_unittest.cc?r1=127717&r2=127716&pathrev=127717

Revert 118788 - Revert 113405 - Revert 113305 - Revert 113300 - Revert 112134 - Revert 112130 - Close idle connections / SPDY sessions when needed

Due to the idle connection state being held by different socket pools, it's possible for one socket pool to hold an idle socket in a lower layer socket pool. From the lower level socket pool's perspective, the socket is being "actively" used. From the higher socket pool's (including SpdySession, which is more of a connection manager) perspective, the connection is idle and can be closed if we have hit a limit.

Normally this isn't a big deal, except when we have a lot of idle SPDY connections and are connecting via a proxy, so we have low connection limits through the proxy server. We address this problem by allowing lower-level socket pools to tell higher level socket pools to close a socket.

Fixed ASAN test failures by removing .Times(1) and .Times(2) from CloseMultipleIdleSocketsHeldByLayeredPoolWhenNeeded unittest (this removes the tests relying on the order of std::set in CloseOneIdleConnectionInLayeredPool). ASAN is prob
ably causing the memory allocator to allocate the pools differently. The std::set is ordered by LayeredPool* which is the address of the LayeredPool (willchan).

Added NET_EXPORT for layered_pool class defintion to fix windows shared compile.

BUG= 62364 ,  92244 ,  109876 ,  110368 

TEST=


Review URL: http://codereview.chromium.org/9667016
------------------------------------------------------------------------
Project Member

Comment 17 by bugdroid1@chromium.org, Mar 20 2012

The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=127730

------------------------------------------------------------------------
r127730 | rch@chromium.org | Tue Mar 20 11:23:25 PDT 2012

Changed paths:
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_pool_base_unittest.cc?r1=127730&r2=127729&pathrev=127730
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_pool_base.h?r1=127730&r2=127729&pathrev=127730
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_pool.h?r1=127730&r2=127729&pathrev=127730
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_pool_base.cc?r1=127730&r2=127729&pathrev=127730
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/spdy/spdy_session.cc?r1=127730&r2=127729&pathrev=127730
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/transport_client_socket_pool.cc?r1=127730&r2=127729&pathrev=127730
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_handle.h?r1=127730&r2=127729&pathrev=127730
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/http/http_network_transaction_spdy2_unittest.cc?r1=127730&r2=127729&pathrev=127730
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/http/http_network_transaction_spdy3_unittest.cc?r1=127730&r2=127729&pathrev=127730
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/http/http_proxy_client_socket_pool.cc?r1=127730&r2=127729&pathrev=127730
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/ssl_client_socket_pool.cc?r1=127730&r2=127729&pathrev=127730
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/socks_client_socket_pool.h?r1=127730&r2=127729&pathrev=127730
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/http/http_network_transaction_spdy21_unittest.cc?r1=127730&r2=127729&pathrev=127730
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/socks_client_socket_pool.cc?r1=127730&r2=127729&pathrev=127730
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/http/http_proxy_client_socket_pool.h?r1=127730&r2=127729&pathrev=127730
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/ssl_client_socket_pool.h?r1=127730&r2=127729&pathrev=127730
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/spdy/spdy_session.h?r1=127730&r2=127729&pathrev=127730
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/spdy/spdy_session_spdy2_unittest.cc?r1=127730&r2=127729&pathrev=127730
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/transport_client_socket_pool.h?r1=127730&r2=127729&pathrev=127730
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_handle.cc?r1=127730&r2=127729&pathrev=127730
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/spdy/spdy_session_spdy3_unittest.cc?r1=127730&r2=127729&pathrev=127730

Revert 127717 - Revert 118788 - Revert 113405 - Revert 113305 - Revert 113300 - Revert 112134 - Revert 112130 - Close idle connections / SPDY sessions when needed

Due to the idle connection state being held by different socket pools, it's possible for one socket pool to hold an idle socket in a lower layer socket pool. From the lower level socket pool's perspective, the socket is being "actively" used. From the higher socket pool's (including SpdySession, which is more of a connection manager) perspective, the connection is idle and can be closed if we have hit a limit.

Normally this isn't a big deal, except when we have a lot of idle SPDY connections and are connecting via a proxy, so we have low connection limits through the proxy server. We address this problem by allowing lower-level socket pools to tell higher level socket pools to close a socket.

Fixed ASAN test failures by removing .Times(1) and .Times(2) from CloseMultipleIdleSocketsHeldByLayeredPoolWhenNeeded unittest (this removes the tests relying on the order of std::set in CloseOneIdleConnectionInLayeredPool). ASAN is prob
ably causing the memory allocator to allocate the pools differently. The std::set is ordered by LayeredPool* which is the address of the LayeredPool (willchan).

Added NET_EXPORT for layered_pool class defintion to fix windows shared compile.

BUG= 62364 ,  92244 ,  109876 ,  110368 

TEST=


Review URL: http://codereview.chromium.org/9667016

TBR=rch@chromium.org
Review URL: https://chromiumcodereview.appspot.com/9760002
------------------------------------------------------------------------
Project Member

Comment 18 by bugdroid1@chromium.org, Mar 21 2012

The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=127893

------------------------------------------------------------------------
r127893 | rch@chromium.org | Tue Mar 20 20:21:00 PDT 2012

Changed paths:
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_pool_base_unittest.cc?r1=127893&r2=127892&pathrev=127893
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_pool_base.h?r1=127893&r2=127892&pathrev=127893
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_pool.h?r1=127893&r2=127892&pathrev=127893
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_pool_base.cc?r1=127893&r2=127892&pathrev=127893
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/spdy/spdy_session.cc?r1=127893&r2=127892&pathrev=127893
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/transport_client_socket_pool.cc?r1=127893&r2=127892&pathrev=127893
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_handle.h?r1=127893&r2=127892&pathrev=127893
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/http/http_network_transaction_spdy2_unittest.cc?r1=127893&r2=127892&pathrev=127893
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/http/http_network_transaction_spdy3_unittest.cc?r1=127893&r2=127892&pathrev=127893
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/http/http_proxy_client_socket_pool.cc?r1=127893&r2=127892&pathrev=127893
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/ssl_client_socket_pool.cc?r1=127893&r2=127892&pathrev=127893
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/socks_client_socket_pool.h?r1=127893&r2=127892&pathrev=127893
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/http/http_network_transaction_spdy21_unittest.cc?r1=127893&r2=127892&pathrev=127893
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/socks_client_socket_pool.cc?r1=127893&r2=127892&pathrev=127893
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/http/http_proxy_client_socket_pool.h?r1=127893&r2=127892&pathrev=127893
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/ssl_client_socket_pool.h?r1=127893&r2=127892&pathrev=127893
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/spdy/spdy_session.h?r1=127893&r2=127892&pathrev=127893
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/spdy/spdy_session_spdy2_unittest.cc?r1=127893&r2=127892&pathrev=127893
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/transport_client_socket_pool.h?r1=127893&r2=127892&pathrev=127893
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_handle.cc?r1=127893&r2=127892&pathrev=127893
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/spdy/spdy_session_spdy3_unittest.cc?r1=127893&r2=127892&pathrev=127893

Attempting to re-land the feature.

Revert 127730 - Revert 127717 - Revert 118788 - Revert 113405 - Revert 113305 - Revert 113300 - Revert 112134 - Revert 112130 - Close idle connections / SPDY sessions when needed

Due to the idle connection state being held by different socket pools, it's possible for one socket pool to hold an idle socket in a lower layer socket pool. From the lower level socket pool's perspective, the socket is being "actively" used. From the higher socket pool's (including SpdySession, which is more of a connection manager) perspective, the connection is idle and can be closed if we have hit a limit.

Normally this isn't a big deal, except when we have a lot of idle SPDY connections and are connecting via a proxy, so we have low connection limits through the proxy server. We address this problem by allowing lower-level socket pools to tell higher level socket pools to close a socket.

Fixed ASAN test failures by removing .Times(1) and .Times(2) from CloseMultipleIdleSocketsHeldByLayeredPoolWhenNeeded unittest (this removes the tests relying on the order of std::set in CloseOneIdleConnectionInLayeredPool). ASAN is prob
ably causing the memory allocator to allocate the pools differently. The std::set is ordered by LayeredPool* which is the address of the LayeredPool (willchan).

Added NET_EXPORT for layered_pool class defintion to fix windows shared compile.

BUG= 62364 ,  92244 ,  109876 ,  110368 

TEST=

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=127717

Review URL: https://chromiumcodereview.appspot.com/9667016
------------------------------------------------------------------------
Project Member

Comment 19 by bugdroid1@chromium.org, Mar 26 2012

The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=129034

------------------------------------------------------------------------
r129034 | rch@chromium.org | Mon Mar 26 16:10:10 PDT 2012

Changed paths:
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_pool_base_unittest.cc?r1=129034&r2=129033&pathrev=129034
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_pool_base.h?r1=129034&r2=129033&pathrev=129034
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_pool.h?r1=129034&r2=129033&pathrev=129034
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_pool_base.cc?r1=129034&r2=129033&pathrev=129034
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/spdy/spdy_session.cc?r1=129034&r2=129033&pathrev=129034
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/transport_client_socket_pool.cc?r1=129034&r2=129033&pathrev=129034
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_handle.h?r1=129034&r2=129033&pathrev=129034
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/http/http_network_transaction_spdy2_unittest.cc?r1=129034&r2=129033&pathrev=129034
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/http/http_network_transaction_spdy3_unittest.cc?r1=129034&r2=129033&pathrev=129034
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/http/http_proxy_client_socket_pool.cc?r1=129034&r2=129033&pathrev=129034
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/ssl_client_socket_pool.cc?r1=129034&r2=129033&pathrev=129034
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/socks_client_socket_pool.h?r1=129034&r2=129033&pathrev=129034
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/http/http_network_transaction_spdy21_unittest.cc?r1=129034&r2=129033&pathrev=129034
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/socks_client_socket_pool.cc?r1=129034&r2=129033&pathrev=129034
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/http/http_proxy_client_socket_pool.h?r1=129034&r2=129033&pathrev=129034
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/ssl_client_socket_pool.h?r1=129034&r2=129033&pathrev=129034
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/spdy/spdy_session.h?r1=129034&r2=129033&pathrev=129034
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/spdy/spdy_session_spdy2_unittest.cc?r1=129034&r2=129033&pathrev=129034
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/transport_client_socket_pool.h?r1=129034&r2=129033&pathrev=129034
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_handle.cc?r1=129034&r2=129033&pathrev=129034
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/spdy/spdy_session_spdy3_unittest.cc?r1=129034&r2=129033&pathrev=129034

Reverting this feature, once again.  *sigh*

Revert 127893 -Revert 127730 - Revert 127717 - Revert 118788 - Revert 113405 - Revert 113305 - Revert 113300 - Revert 112134 - Revert 112130 - Close idle connections / SPDY sessions when needed

Due to the idle connection state being held by different socket pools, it's possible for one socket pool to hold an idle socket in a lower layer socket pool. From the lower level socket pool's perspective, the socket is being "actively" used. From the higher socket pool's (including SpdySession, which is more of a connection manager) perspective, the connection is idle and can be closed if we have hit a limit.

Normally this isn't a big deal, except when we have a lot of idle SPDY connections and are connecting via a proxy, so we have low connection limits through the proxy server. We address this problem by allowing lower-level socket pools to tell higher level socket pools to close a socket.

Fixed ASAN test failures by removing .Times(1) and .Times(2) from CloseMultipleIdleSocketsHeldByLayeredPoolWhenNeeded unittest (this removes the tests relying on the order of std::set in CloseOneIdleConnectionInLayeredPool). ASAN is prob
ably causing the memory allocator to allocate the pools differently. The std::set is ordered by LayeredPool* which is the address of the LayeredPool (willchan).

Added NET_EXPORT for layered_pool class defintion to fix windows shared compile.

BUG= 62364 ,  92244 ,  109876 ,  110368 ,  119847 
TEST=

Review URL: http://codereview.chromium.org/9809033
------------------------------------------------------------------------

Comment 20 by laforge@google.com, Mar 27 2012

Labels: -Mstone-19 Mstone-20 MovedFrom-19
Project Member

Comment 21 by bugdroid1@chromium.org, Mar 27 2012

Labels: merge-merged-1081
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=129277

------------------------------------------------------------------------
r129277 | dharani@chromium.org | Tue Mar 27 14:36:45 PDT 2012

Changed paths:
 M http://src.chromium.org/viewvc/chrome/branches/1081/src/net/socket/client_socket_pool.h?r1=129277&r2=129276&pathrev=129277
 M http://src.chromium.org/viewvc/chrome/branches/1081/src/net/socket/client_socket_pool_base.cc?r1=129277&r2=129276&pathrev=129277
 M http://src.chromium.org/viewvc/chrome/branches/1081/src/net/socket/transport_client_socket_pool.cc?r1=129277&r2=129276&pathrev=129277
 M http://src.chromium.org/viewvc/chrome/branches/1081/src/net/socket/ssl_client_socket_pool.cc?r1=129277&r2=129276&pathrev=129277
 M http://src.chromium.org/viewvc/chrome/branches/1081/src/net/http/http_network_transaction_spdy21_unittest.cc?r1=129277&r2=129276&pathrev=129277
 M http://src.chromium.org/viewvc/chrome/branches/1081/src/net/socket/client_socket_handle.h?r1=129277&r2=129276&pathrev=129277
 M http://src.chromium.org/viewvc/chrome/branches/1081/src/net/socket/socks_client_socket_pool.cc?r1=129277&r2=129276&pathrev=129277
 M http://src.chromium.org/viewvc/chrome/branches/1081/src/net/http/http_network_transaction_spdy2_unittest.cc?r1=129277&r2=129276&pathrev=129277
 M http://src.chromium.org/viewvc/chrome/branches/1081/src/net/http/http_proxy_client_socket_pool.h?r1=129277&r2=129276&pathrev=129277
 M http://src.chromium.org/viewvc/chrome/branches/1081/src/net/http/http_network_transaction_spdy3_unittest.cc?r1=129277&r2=129276&pathrev=129277
 M http://src.chromium.org/viewvc/chrome/branches/1081/src/net/http/http_proxy_client_socket_pool.cc?r1=129277&r2=129276&pathrev=129277
 M http://src.chromium.org/viewvc/chrome/branches/1081/src/net/socket/client_socket_handle.cc?r1=129277&r2=129276&pathrev=129277
 M http://src.chromium.org/viewvc/chrome/branches/1081/src/net/socket/socks_client_socket_pool.h?r1=129277&r2=129276&pathrev=129277
 M http://src.chromium.org/viewvc/chrome/branches/1081/src/net/socket/ssl_client_socket_pool.h?r1=129277&r2=129276&pathrev=129277
 M http://src.chromium.org/viewvc/chrome/branches/1081/src/net/spdy/spdy_session_spdy2_unittest.cc?r1=129277&r2=129276&pathrev=129277
 M http://src.chromium.org/viewvc/chrome/branches/1081/src/net/socket/transport_client_socket_pool.h?r1=129277&r2=129276&pathrev=129277
 M http://src.chromium.org/viewvc/chrome/branches/1081/src/net/spdy/spdy_session.h?r1=129277&r2=129276&pathrev=129277
 M http://src.chromium.org/viewvc/chrome/branches/1081/src/net/spdy/spdy_session_spdy3_unittest.cc?r1=129277&r2=129276&pathrev=129277
 M http://src.chromium.org/viewvc/chrome/branches/1081/src/net/socket/client_socket_pool_base_unittest.cc?r1=129277&r2=129276&pathrev=129277
 M http://src.chromium.org/viewvc/chrome/branches/1081/src/net/socket/client_socket_pool_base.h?r1=129277&r2=129276&pathrev=129277

Revert 127893 - Attempting to re-land the feature.

Revert 127730 - Revert 127717 - Revert 118788 - Revert 113405 - Revert 113305 - Revert 113300 - Revert 112134 - Revert 112130 - Close idle connections / SPDY sessions when needed

Due to the idle connection state being held by different socket pools, it's possible for one socket pool to hold an idle socket in a lower layer socket pool. From the lower level socket pool's perspective, the socket is being "actively" used. From the higher socket pool's (including SpdySession, which is more of a connection manager) perspective, the connection is idle and can be closed if we have hit a limit.

Normally this isn't a big deal, except when we have a lot of idle SPDY connections and are connecting via a proxy, so we have low connection limits through the proxy server. We address this problem by allowing lower-level socket pools to tell higher level socket pools to close a socket.

Fixed ASAN test failures by removing .Times(1) and .Times(2) from CloseMultipleIdleSocketsHeldByLayeredPoolWhenNeeded unittest (this removes the tests relying on the order of std::set in CloseOneIdleConnectionInLayeredPool). ASAN is prob
ably causing the memory allocator to allocate the pools differently. The std::set is ordered by LayeredPool* which is the address of the LayeredPool (willchan).

Added NET_EXPORT for layered_pool class defintion to fix windows shared compile.

BUG= 62364 ,  92244 ,  109876 ,  110368 

TEST=

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=127717

Review URL: https://chromiumcodereview.appspot.com/9667016

TBR=rch@chromium.org
------------------------------------------------------------------------
Project Member

Comment 22 by bugdroid1@chromium.org, Mar 27 2012

The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=129293

------------------------------------------------------------------------
r129293 | dharani@chromium.org | Tue Mar 27 15:51:41 PDT 2012

Changed paths:
 M http://src.chromium.org/viewvc/chrome/branches/1081/src/net/socket/client_socket_pool.h?r1=129293&r2=129292&pathrev=129293
 M http://src.chromium.org/viewvc/chrome/branches/1081/src/net/socket/client_socket_pool_base.cc?r1=129293&r2=129292&pathrev=129293
 M http://src.chromium.org/viewvc/chrome/branches/1081/src/net/socket/transport_client_socket_pool.cc?r1=129293&r2=129292&pathrev=129293
 M http://src.chromium.org/viewvc/chrome/branches/1081/src/net/socket/ssl_client_socket_pool.cc?r1=129293&r2=129292&pathrev=129293
 M http://src.chromium.org/viewvc/chrome/branches/1081/src/net/http/http_network_transaction_spdy21_unittest.cc?r1=129293&r2=129292&pathrev=129293
 M http://src.chromium.org/viewvc/chrome/branches/1081/src/net/socket/client_socket_handle.h?r1=129293&r2=129292&pathrev=129293
 M http://src.chromium.org/viewvc/chrome/branches/1081/src/net/socket/socks_client_socket_pool.cc?r1=129293&r2=129292&pathrev=129293
 M http://src.chromium.org/viewvc/chrome/branches/1081/src/net/http/http_network_transaction_spdy2_unittest.cc?r1=129293&r2=129292&pathrev=129293
 M http://src.chromium.org/viewvc/chrome/branches/1081/src/net/http/http_proxy_client_socket_pool.h?r1=129293&r2=129292&pathrev=129293
 M http://src.chromium.org/viewvc/chrome/branches/1081/src/net/http/http_network_transaction_spdy3_unittest.cc?r1=129293&r2=129292&pathrev=129293
 M http://src.chromium.org/viewvc/chrome/branches/1081/src/net/http/http_proxy_client_socket_pool.cc?r1=129293&r2=129292&pathrev=129293
 M http://src.chromium.org/viewvc/chrome/branches/1081/src/net/socket/client_socket_handle.cc?r1=129293&r2=129292&pathrev=129293
 M http://src.chromium.org/viewvc/chrome/branches/1081/src/net/socket/socks_client_socket_pool.h?r1=129293&r2=129292&pathrev=129293
 M http://src.chromium.org/viewvc/chrome/branches/1081/src/net/socket/ssl_client_socket_pool.h?r1=129293&r2=129292&pathrev=129293
 M http://src.chromium.org/viewvc/chrome/branches/1081/src/net/spdy/spdy_session_spdy2_unittest.cc?r1=129293&r2=129292&pathrev=129293
 M http://src.chromium.org/viewvc/chrome/branches/1081/src/net/socket/transport_client_socket_pool.h?r1=129293&r2=129292&pathrev=129293
 M http://src.chromium.org/viewvc/chrome/branches/1081/src/net/spdy/spdy_session.h?r1=129293&r2=129292&pathrev=129293
 M http://src.chromium.org/viewvc/chrome/branches/1081/src/net/spdy/spdy_session_spdy3_unittest.cc?r1=129293&r2=129292&pathrev=129293
 M http://src.chromium.org/viewvc/chrome/branches/1081/src/net/socket/client_socket_pool_base_unittest.cc?r1=129293&r2=129292&pathrev=129293
 M http://src.chromium.org/viewvc/chrome/branches/1081/src/net/socket/client_socket_pool_base.h?r1=129293&r2=129292&pathrev=129293

Revert 129277 - Revert 127893 - Attempting to re-land the feature.

Revert 127730 - Revert 127717 - Revert 118788 - Revert 113405 - Revert 113305 - Revert 113300 - Revert 112134 - Revert 112130 - Close idle connections / SPDY sessions when needed

Due to the idle connection state being held by different socket pools, it's possible for one socket pool to hold an idle socket in a lower layer socket pool. From the lower level socket pool's perspective, the socket is being "actively" used. From the higher socket pool's (including SpdySession, which is more of a connection manager) perspective, the connection is idle and can be closed if we have hit a limit.

Normally this isn't a big deal, except when we have a lot of idle SPDY connections and are connecting via a proxy, so we have low connection limits through the proxy server. We address this problem by allowing lower-level socket pools to tell higher level socket pools to close a socket.

Fixed ASAN test failures by removing .Times(1) and .Times(2) from CloseMultipleIdleSocketsHeldByLayeredPoolWhenNeeded unittest (this removes the tests relying on the order of std::set in CloseOneIdleConnectionInLayeredPool). ASAN is prob
ably causing the memory allocator to allocate the pools differently. The std::set is ordered by LayeredPool* which is the address of the LayeredPool (willchan).

Added NET_EXPORT for layered_pool class defintion to fix windows shared compile.

BUG= 62364 ,  92244 ,  109876 ,  110368 

TEST=

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=127717

Review URL: https://chromiumcodereview.appspot.com/9667016

TBR=rch@chromium.org

TBR=dharani@chromium.org
Review URL: https://chromiumcodereview.appspot.com/9791044
------------------------------------------------------------------------
Project Member

Comment 23 by bugdroid1@chromium.org, Mar 27 2012

The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=129294

------------------------------------------------------------------------
r129294 | rch@chromium.org | Tue Mar 27 16:01:38 PDT 2012

Changed paths:
 M http://src.chromium.org/viewvc/chrome/branches/1081/src/net/socket/client_socket_pool.h?r1=129294&r2=129293&pathrev=129294
 M http://src.chromium.org/viewvc/chrome/branches/1081/src/net/socket/client_socket_pool_base.cc?r1=129294&r2=129293&pathrev=129294
 M http://src.chromium.org/viewvc/chrome/branches/1081/src/net/socket/transport_client_socket_pool.cc?r1=129294&r2=129293&pathrev=129294
 M http://src.chromium.org/viewvc/chrome/branches/1081/src/net/spdy/spdy_session.cc?r1=129294&r2=129293&pathrev=129294
 M http://src.chromium.org/viewvc/chrome/branches/1081/src/net/socket/ssl_client_socket_pool.cc?r1=129294&r2=129293&pathrev=129294
 M http://src.chromium.org/viewvc/chrome/branches/1081/src/net/http/http_network_transaction_spdy21_unittest.cc?r1=129294&r2=129293&pathrev=129294
 M http://src.chromium.org/viewvc/chrome/branches/1081/src/net/socket/client_socket_handle.h?r1=129294&r2=129293&pathrev=129294
 M http://src.chromium.org/viewvc/chrome/branches/1081/src/net/socket/socks_client_socket_pool.cc?r1=129294&r2=129293&pathrev=129294
 M http://src.chromium.org/viewvc/chrome/branches/1081/src/net/http/http_network_transaction_spdy2_unittest.cc?r1=129294&r2=129293&pathrev=129294
 M http://src.chromium.org/viewvc/chrome/branches/1081/src/net/http/http_proxy_client_socket_pool.h?r1=129294&r2=129293&pathrev=129294
 M http://src.chromium.org/viewvc/chrome/branches/1081/src/net/http/http_network_transaction_spdy3_unittest.cc?r1=129294&r2=129293&pathrev=129294
 M http://src.chromium.org/viewvc/chrome/branches/1081/src/net/http/http_proxy_client_socket_pool.cc?r1=129294&r2=129293&pathrev=129294
 M http://src.chromium.org/viewvc/chrome/branches/1081/src/net/socket/client_socket_handle.cc?r1=129294&r2=129293&pathrev=129294
 M http://src.chromium.org/viewvc/chrome/branches/1081/src/net/socket/socks_client_socket_pool.h?r1=129294&r2=129293&pathrev=129294
 M http://src.chromium.org/viewvc/chrome/branches/1081/src/net/socket/ssl_client_socket_pool.h?r1=129294&r2=129293&pathrev=129294
 M http://src.chromium.org/viewvc/chrome/branches/1081/src/net/spdy/spdy_session_spdy2_unittest.cc?r1=129294&r2=129293&pathrev=129294
 M http://src.chromium.org/viewvc/chrome/branches/1081/src/net/socket/transport_client_socket_pool.h?r1=129294&r2=129293&pathrev=129294
 M http://src.chromium.org/viewvc/chrome/branches/1081/src/net/spdy/spdy_session.h?r1=129294&r2=129293&pathrev=129294
 M http://src.chromium.org/viewvc/chrome/branches/1081/src/net/spdy/spdy_session_spdy3_unittest.cc?r1=129294&r2=129293&pathrev=129294
 M http://src.chromium.org/viewvc/chrome/branches/1081/src/net/socket/client_socket_pool_base_unittest.cc?r1=129294&r2=129293&pathrev=129294
 M http://src.chromium.org/viewvc/chrome/branches/1081/src/net/socket/client_socket_pool_base.h?r1=129294&r2=129293&pathrev=129294

Revert 127893 - Attempting to re-land the feature.

Revert 127730 - Revert 127717 - Revert 118788 - Revert 113405 - Revert 113305 - Revert 113300 - Revert 112134 - Revert 112130 - Close idle connections / SPDY sessions when needed

Due to the idle connection state being held by different socket pools, it's possible for one socket pool to hold an idle socket in a lower layer socket pool. From the lower level socket pool's perspective, the socket is being "actively" used. From the higher socket pool's (including SpdySession, which is more of a connection manager) perspective, the connection is idle and can be closed if we have hit a limit.

Normally this isn't a big deal, except when we have a lot of idle SPDY connections and are connecting via a proxy, so we have low connection limits through the proxy server. We address this problem by allowing lower-level socket pools to tell higher level socket pools to close a socket.

Fixed ASAN test failures by removing .Times(1) and .Times(2) from CloseMultipleIdleSocketsHeldByLayeredPoolWhenNeeded unittest (this removes the tests relying on the order of std::set in CloseOneIdleConnectionInLayeredPool). ASAN is prob
ably causing the memory allocator to allocate the pools differently. The std::set is ordered by LayeredPool* which is the address of the LayeredPool (willchan).

Added NET_EXPORT for layered_pool class defintion to fix windows shared compile.

BUG= 62364 ,  92244 ,  109876 ,  110368 

TEST=

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=127717

Review URL: https://chromiumcodereview.appspot.com/9667016

TBR=rch@chromium.org
Review URL: https://chromiumcodereview.appspot.com/9844007
------------------------------------------------------------------------
Project Member

Comment 24 by bugdroid1@chromium.org, Apr 2 2012

The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=130129

------------------------------------------------------------------------
r130129 | rch@chromium.org | Mon Apr 02 08:22:43 PDT 2012

Changed paths:
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_pool_base_unittest.cc?r1=130129&r2=130128&pathrev=130129
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_pool_base.h?r1=130129&r2=130128&pathrev=130129
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_pool.h?r1=130129&r2=130128&pathrev=130129
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_pool.cc?r1=130129&r2=130128&pathrev=130129
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_pool_base.cc?r1=130129&r2=130128&pathrev=130129
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/spdy/spdy_session.cc?r1=130129&r2=130128&pathrev=130129
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/transport_client_socket_pool.cc?r1=130129&r2=130128&pathrev=130129
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_handle.h?r1=130129&r2=130128&pathrev=130129
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/http/http_network_transaction_spdy2_unittest.cc?r1=130129&r2=130128&pathrev=130129
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/http/http_network_transaction_spdy3_unittest.cc?r1=130129&r2=130128&pathrev=130129
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/http/http_proxy_client_socket_pool.cc?r1=130129&r2=130128&pathrev=130129
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/ssl_client_socket_pool.cc?r1=130129&r2=130128&pathrev=130129
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/socks_client_socket_pool.h?r1=130129&r2=130128&pathrev=130129
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/http/http_network_transaction_spdy21_unittest.cc?r1=130129&r2=130128&pathrev=130129
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/socks_client_socket_pool.cc?r1=130129&r2=130128&pathrev=130129
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/http/http_proxy_client_socket_pool.h?r1=130129&r2=130128&pathrev=130129
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/ssl_client_socket_pool.h?r1=130129&r2=130128&pathrev=130129
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/spdy/spdy_session.h?r1=130129&r2=130128&pathrev=130129
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/spdy/spdy_session_spdy2_unittest.cc?r1=130129&r2=130128&pathrev=130129
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/transport_client_socket_pool.h?r1=130129&r2=130128&pathrev=130129
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_handle.cc?r1=130129&r2=130128&pathrev=130129
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/spdy/spdy_session_spdy3_unittest.cc?r1=130129&r2=130128&pathrev=130129

Attempting to re-land this feature with instrumentation to track down the use-after-free.

Revert 129034 - Revert 127893 - Revert 127730 - Revert 127717 - Revert 118788 - Revert 113405 - Revert 113305 - Revert 113300 - Revert 112134 - Revert 112130 - Close idle connections / SPDY sessions when needed

Due to the idle connection state being held by different socket pools, it's possible for one socket pool to hold an idle socket in a lower layer socket pool. From the lower level socket pool's perspective, the socket is being "actively" used. From the higher socket pool's (including SpdySession, which is more of a connection manager) perspective, the connection is idle and can be closed if we have hit a limit.

Normally this isn't a big deal, except when we have a lot of idle SPDY connections and are connecting via a proxy, so we have low connection limits through the proxy server. We address this problem by allowing lower-level socket pools to tell higher level socket pools to close a socket.

Fixed ASAN test failures by removing .Times(1) and .Times(2) from CloseMultipleIdleSocketsHeldByLayeredPoolWhenNeeded unittest (this removes the tests relying on the order of std::set in CloseOneIdleConnectionInLayeredPool). ASAN is prob
ably causing the memory allocator to allocate the pools differently. The std::set is ordered by LayeredPool* which is the address of the LayeredPool (willchan).

Added NET_EXPORT for layered_pool class defintion to fix windows shared compile.

BUG= 62364 ,  92244 ,  109876 ,  110368 ,  119847 
TEST=


Review URL: http://codereview.chromium.org/9861032
------------------------------------------------------------------------
Project Member

Comment 25 by bugdroid1@chromium.org, Apr 6 2012

The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=131145

------------------------------------------------------------------------
r131145 | rch@chromium.org | Fri Apr 06 10:26:40 PDT 2012

Changed paths:
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_pool_base_unittest.cc?r1=131145&r2=131144&pathrev=131145
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_pool_base.h?r1=131145&r2=131144&pathrev=131145
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_pool.h?r1=131145&r2=131144&pathrev=131145
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_pool.cc?r1=131145&r2=131144&pathrev=131145
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_pool_base.cc?r1=131145&r2=131144&pathrev=131145
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/spdy/spdy_session.cc?r1=131145&r2=131144&pathrev=131145
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/transport_client_socket_pool.cc?r1=131145&r2=131144&pathrev=131145
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_handle.h?r1=131145&r2=131144&pathrev=131145
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/http/http_network_transaction_spdy2_unittest.cc?r1=131145&r2=131144&pathrev=131145
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/http/http_network_transaction_spdy3_unittest.cc?r1=131145&r2=131144&pathrev=131145
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/http/http_proxy_client_socket_pool.cc?r1=131145&r2=131144&pathrev=131145
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/ssl_client_socket_pool.cc?r1=131145&r2=131144&pathrev=131145
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/socks_client_socket_pool.h?r1=131145&r2=131144&pathrev=131145
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/http/http_network_transaction_spdy21_unittest.cc?r1=131145&r2=131144&pathrev=131145
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/socks_client_socket_pool.cc?r1=131145&r2=131144&pathrev=131145
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/http/http_proxy_client_socket_pool.h?r1=131145&r2=131144&pathrev=131145
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/ssl_client_socket_pool.h?r1=131145&r2=131144&pathrev=131145
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/spdy/spdy_session.h?r1=131145&r2=131144&pathrev=131145
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/spdy/spdy_session_spdy2_unittest.cc?r1=131145&r2=131144&pathrev=131145
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/transport_client_socket_pool.h?r1=131145&r2=131144&pathrev=131145
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_handle.cc?r1=131145&r2=131144&pathrev=131145
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/spdy/spdy_session_spdy3_unittest.cc?r1=131145&r2=131144&pathrev=131145

Reverting again ... More crashes, and the instrumentation did not appear to help

Revert 130129 - Revert 129034 - Revert 127893 - Revert 127730 - Revert 127717 - Revert 118788 - Revert 113405 - Revert 113305 - Revert 113300 - Revert 112134 - Revert 112130 - Close idle connections / SPDY sessions when needed

Due to the idle connection state being held by different socket pools, it's possible for one socket pool to hold an idle socket in a lower layer socket pool. From the lower level socket pool's perspective, the socket is being "actively" used. From the higher socket pool's (including SpdySession, which is more of a connection manager) perspective, the connection is idle and can be closed if we have hit a limit.

Normally this isn't a big deal, except when we have a lot of idle SPDY connections and are connecting via a proxy, so we have low connection limits through the proxy server. We address this problem by allowing lower-level socket pools to tell higher level socket pools to close a socket.

Fixed ASAN test failures by removing .Times(1) and .Times(2) from CloseMultipleIdleSocketsHeldByLayeredPoolWhenNeeded unittest (this removes the tests relying on the order of std::set in CloseOneIdleConnectionInLayeredPool). ASAN is prob
ably causing the memory allocator to allocate the pools differently. The std::set is ordered by LayeredPool* which is the address of the LayeredPool (willchan).

Added NET_EXPORT for layered_pool class defintion to fix windows shared compile.

BUG= 62364 ,  92244 ,  109876 ,  110368 ,  119847 
TEST=


Review URL: http://codereview.chromium.org/9861032

TBR=rch@chromium.org
Review URL: https://chromiumcodereview.appspot.com/10006036
------------------------------------------------------------------------
Project Member

Comment 26 by bugdroid1@chromium.org, Apr 10 2012

The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=131604

------------------------------------------------------------------------
r131604 | rch@chromium.org | Tue Apr 10 12:19:48 PDT 2012

Changed paths:
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_pool_base_unittest.cc?r1=131604&r2=131603&pathrev=131604
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_pool_base.h?r1=131604&r2=131603&pathrev=131604
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/http/http_proxy_client_socket_pool.cc?r1=131604&r2=131603&pathrev=131604
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/ssl_client_socket_pool.cc?r1=131604&r2=131603&pathrev=131604
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_pool.h?r1=131604&r2=131603&pathrev=131604
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/socks_client_socket_pool.h?r1=131604&r2=131603&pathrev=131604
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_pool.cc?r1=131604&r2=131603&pathrev=131604
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/socks_client_socket_pool.cc?r1=131604&r2=131603&pathrev=131604
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/http/http_proxy_client_socket_pool.h?r1=131604&r2=131603&pathrev=131604
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/ssl_client_socket_pool.h?r1=131604&r2=131603&pathrev=131604
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_pool_base.cc?r1=131604&r2=131603&pathrev=131604
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/transport_client_socket_pool.h?r1=131604&r2=131603&pathrev=131604
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_handle.cc?r1=131604&r2=131603&pathrev=131604
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/transport_client_socket_pool.cc?r1=131604&r2=131603&pathrev=131604
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_handle.h?r1=131604&r2=131603&pathrev=131604

Attempting to re-land a small portion of this change...  Simply add links from 
lower layer pools to higher layer pool.

Revert 10006036 - Revert 130129 - Revert 129034 - Revert 127893 - Revert 127730 - Revert 127717 - Revert 118788 - Revert 113405 - Revert 113305 - Revert 113300 - Revert 112134 - Revert 112130 - Close idle connections / SPDY sessions when needed

Due to the idle connection state being held by different socket pools, it's possible for one socket pool to hold an idle socket in a lower layer socket pool. From the lower level socket pool's perspective, the socket is being "actively" used. From the higher socket pool's (including SpdySession, which is more of a connection manager) perspective, the connection is idle and can be closed if we have hit a limit.

Normally this isn't a big deal, except when we have a lot of idle SPDY connections and are connecting via a proxy, so we have low connection limits through the proxy server. We address this problem by allowing lower-level socket pools to tell higher level socket pools to close a socket.

Fixed ASAN test failures by removing .Times(1) and .Times(2) from CloseMultipleIdleSocketsHeldByLayeredPoolWhenNeeded unittest (this removes the tests relying on the order of std::set in CloseOneIdleConnectionInLayeredPool). ASAN is prob
ably causing the memory allocator to allocate the pools differently. The std::set is ordered by LayeredPool* which is the address of the LayeredPool (willchan).

Added NET_EXPORT for layered_pool class defintion to fix windows shared compile.

BUG= 62364 ,  92244 ,  109876 ,  110368 ,  119847 
TEST=


Review URL: http://codereview.chromium.org/10026024
------------------------------------------------------------------------
Project Member

Comment 27 by bugdroid1@chromium.org, Apr 16 2012

The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=132478

------------------------------------------------------------------------
r132478 | rch@chromium.org | Mon Apr 16 15:37:32 PDT 2012

Changed paths:
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_pool_base_unittest.cc?r1=132478&r2=132477&pathrev=132478
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_pool_base.cc?r1=132478&r2=132477&pathrev=132478

Attempt to close idle connections in higher layer socket pools when a lower layer pool is stalled.

BUG= 62364 ,  92244 
TEST=ClientSocketPoolBaseTest.\*CloseIdleSocket\*


Review URL: http://codereview.chromium.org/9999030
------------------------------------------------------------------------
Project Member

Comment 28 by bugdroid1@chromium.org, Apr 17 2012

The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=132620

------------------------------------------------------------------------
r132620 | rch@chromium.org | Tue Apr 17 12:25:46 PDT 2012

Changed paths:
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_pool_base_unittest.cc?r1=132620&r2=132619&pathrev=132620
 M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_pool_base.cc?r1=132620&r2=132619&pathrev=132620

Revert 132478 - Attempt to close idle connections in higher layer socket pools when a lower layer pool is stalled.

BUG= 62364 ,  92244 
TEST=ClientSocketPoolBaseTest.\*CloseIdleSocket\*


Review URL: http://codereview.chromium.org/9999030

TBR=rch@chromium.org
Review URL: https://chromiumcodereview.appspot.com/10083024
------------------------------------------------------------------------
Labels: -Mstone-20 bulkmove MovedFrom-20 Mstone-21
M20 is about to sail in couple of days. If this should be part of M20, add it back.
Labels: -Mstone-21 Mstone-22
Owner: rch@chromium.org
Status: Assigned
I believe this will be fixed as part of the idle SpdySession fix that rch@ is doing, that probably won't make Mstone-21.

Comment 31 by k...@google.com, Oct 2 2012

Labels: -Mstone-22 Mstone-24 MovedFrom-22
Moving out to M24, Please pull back in to previous milestones if needed.
Labels: -Mstone-24
Since the bug has moved few times, removing the milestone label. Please target the right milestone.
Cc: apps-tses-bugs@chromium.org cyrusm@chromium.org
Labels: Hotlist-Enterprise
What is the status of this bug ? 

This may be root cause for significant speed impact in enterprises which use spdy enabled proxies [ crbug/146646 ]. zscaler.com is a proxy service which lot of schools/enterprises use... which is spdy enabled and has been involved in some of the issues reported to us. Feel free to contact me directly for detailed net-internals  dump if further analysis is needed.

No one's had a chance to look into the reason for the breakages that resulted from rch's last patch that attempted to fix this.  The issue is still on our radar, though.  Both rtenneti and I are interested in picking it up again, when we have time.  Not sure who will get to it first (And no ETA).
Owner: mmenke@chromium.org
I talked to Matt and would rather he take it on. I'm going to talk to Raman tomorrow. I want him to focus on our upcoming SPDY work.
Project Member

Comment 36 by bugdroid1@chromium.org, Mar 10 2013

Labels: -Area-Internals -Internals-Network-Proxy -Internals-Network-HTTP Cr-Internals-Network-Proxy Cr-Internals-Network-HTTP Cr-Internals
Labels: -MovedFrom-17 -MovedFrom18 -MovedFrom-19 -merge-merged-1081 -bulkmove -MovedFrom-20 -MovedFrom-22 -Cr-Internals
Status: Started
I think I've figured out at least two problems with the reverted implementations.

1)  Closing idle sockets in higher level pools would trigger trying to create new connections in the current pool, including for the group we are currently looking at.  I can't see how this would cause a crash unless the connection attempts synchronously failed, resulting in deleting the current group, but it could result in more connections than pending requests for a group.  Eric landed a fix for crashes in the case the group was missing, though I'm not sure this happens often enough for this to be the primary cause of the crashes Eric fixed.

2)  Another problem is that when a socket becomes idle in a higher level group, we don't check if a lower level group could use an extra socket slot.  I also don't see how this could cause a crash, however.

So...two issues, neither of which seems to explain crashes, other than one fixed crash, though it's certainly possible I'm missing some way they could be responsible.
3)  When we close idle sockets in higher level pools in ProcessPendingRequest, things are a bit worse than RequestSocket.  The socket is already in a group, closing a socket in a higher layered pool may result in try to service the same request again, at the bottom of the stack.  This is bad and weird.

4)  When a higher layered pool sends a request to a lower level pool, the lower level pool may try to close an idle socket in the higher level pool.  If this succeeds, and it's a call from ProcessPendingRequest at the higher level pool (But RequestSocket at the lower level pool), we may get into a similar situation.  Given that this only happens when we just closed a socket in the higher level pool, this case is less likely, but we still should handle it.

Given the monstrosity that is our socket pools, I think the only thing we can reasonably do without restructuring them is just to close idle sockets in higher layered pools asynchronously.  This is simple and should pretty much eliminate the problem.  If we go with the way the reverted patches went instead, I'm afraid we'd end up firefighting these re-entrancy issues for quite a while, unless we made sure that closing an idle socket in a higher layered pool didn't trigger the normal socket slot reuse logic.

I have a very simple CL to do this.  The one big ugliness is it adds yet another function to socket pool connection logic that's called at a separate time from the others.  The code doesn't actually try to use the sockets - closing the sockets should call back into the current pool as needed.  I think this is the way to go, but even if others disagree, I'd still like to land this to verify it actually circumvents all the crash issues we were seeing before, and there's no other issue with the use of LayeredPools.

Even landing this patch won't completely fix the original issue - when higher layered pools get an idle socket, they still need to inform lower level pools of this fact, but I'll worry about that later.
Project Member

Comment 40 by bugdroid1@chromium.org, Apr 22 2013

------------------------------------------------------------------------
r195540 | mmenke@chromium.org | 2013-04-22T17:32:20.878581Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_pool_base_unittest.cc?r1=195540&r2=195539&pathrev=195540
   M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_pool_base.cc?r1=195540&r2=195539&pathrev=195540
   M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_pool_base.h?r1=195540&r2=195539&pathrev=195540

When a new request comes in to a socket pool, and that
request's group is stalled due to the pool hitting the
connection limit, try to close idle sockets in higher
level pools.

CLs to do this have been landed and reverted in the
past, due to crashes.  This CL attempts to avoid such
crashes by reducing re-entracy.  Idle sockets are closed
asynchronously, and the CL relies on that automatically
triggering use of the freed up socket slot.

BUG= 92244 

Review URL: https://codereview.chromium.org/14308013
------------------------------------------------------------------------
Project Member

Comment 42 by bugdroid1@chromium.org, Aug 22 2013

------------------------------------------------------------------------
r219147 | mmenke@chromium.org | 2013-08-22T23:41:52.410933Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/transport_client_socket_pool.cc?r1=219147&r2=219146&pathrev=219147
   M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/transport_client_socket_pool.h?r1=219147&r2=219146&pathrev=219147
   M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/ssl_client_socket_pool.cc?r1=219147&r2=219146&pathrev=219147
   M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_handle.cc?r1=219147&r2=219146&pathrev=219147
   M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/ssl_client_socket_pool.h?r1=219147&r2=219146&pathrev=219147
   M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_handle.h?r1=219147&r2=219146&pathrev=219147
   M http://src.chromium.org/viewvc/chrome/trunk/src/net/http/http_proxy_client_socket_pool.cc?r1=219147&r2=219146&pathrev=219147
   M http://src.chromium.org/viewvc/chrome/trunk/src/net/spdy/spdy_session_unittest.cc?r1=219147&r2=219146&pathrev=219147
   M http://src.chromium.org/viewvc/chrome/trunk/src/net/http/http_proxy_client_socket_pool.h?r1=219147&r2=219146&pathrev=219147
   M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_pool_base_unittest.cc?r1=219147&r2=219146&pathrev=219147
   M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_pool_base.cc?r1=219147&r2=219146&pathrev=219147
   M http://src.chromium.org/viewvc/chrome/trunk/src/net/spdy/spdy_session.cc?r1=219147&r2=219146&pathrev=219147
   M http://src.chromium.org/viewvc/chrome/trunk/src/net/spdy/spdy_session.h?r1=219147&r2=219146&pathrev=219147
   M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_pool_base.h?r1=219147&r2=219146&pathrev=219147
   M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/client_socket_pool.h?r1=219147&r2=219146&pathrev=219147
   M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/socks_client_socket_pool.cc?r1=219147&r2=219146&pathrev=219147
   M http://src.chromium.org/viewvc/chrome/trunk/src/net/http/http_network_transaction_unittest.cc?r1=219147&r2=219146&pathrev=219147
   M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/socks_client_socket_pool.h?r1=219147&r2=219146&pathrev=219147

When an idle socket is added back to a socket pool,
check if lower layer socket pools are stalled.  If so,
close the idle socket.

Also, when a SPDY stream is destroyed, check if
the session is idle and a lower layer pool is stalled,
and close the session if needed.

BUG= 92244 

Review URL: https://chromiumcodereview.appspot.com/18796003
------------------------------------------------------------------------
Status: Fixed
Components: Internals>Network
Components: -Internals>Network>HTTP

Sign in to add a comment