summaryrefslogtreecommitdiffstats
path: root/net
diff options
context:
space:
mode:
Diffstat (limited to 'net')
-rw-r--r--net/base/client_socket.h5
-rw-r--r--net/base/client_socket_pool.cc15
-rw-r--r--net/base/client_socket_pool.h17
-rw-r--r--net/base/client_socket_pool_unittest.cc5
-rw-r--r--net/base/ssl_client_socket_mac.cc11
-rw-r--r--net/base/ssl_client_socket_mac.h1
-rw-r--r--net/base/ssl_client_socket_nss.cc20
-rw-r--r--net/base/ssl_client_socket_nss.h1
-rw-r--r--net/base/ssl_client_socket_unittest.cc5
-rw-r--r--net/base/ssl_client_socket_win.cc11
-rw-r--r--net/base/ssl_client_socket_win.h1
-rw-r--r--net/base/tcp_client_socket.h1
-rw-r--r--net/base/tcp_client_socket_libevent.cc16
-rw-r--r--net/base/tcp_client_socket_unittest.cc4
-rw-r--r--net/base/tcp_client_socket_win.cc16
-rw-r--r--net/http/http_network_transaction_unittest.cc3
16 files changed, 118 insertions, 14 deletions
diff --git a/net/base/client_socket.h b/net/base/client_socket.h
index b96f07b..f61c7f5 100644
--- a/net/base/client_socket.h
+++ b/net/base/client_socket.h
@@ -47,6 +47,11 @@ class ClientSocket : public Socket {
// connection wasn't established or the connection is dead.
virtual bool IsConnected() const = 0;
+ // Called to test if the connection is still alive and idle. Returns false
+ // if a connection wasn't established, the connection is dead, or some data
+ // have been received.
+ virtual bool IsConnectedAndIdle() const = 0;
+
#if defined(OS_LINUX)
// Identical to posix system call getpeername().
// Needed by ssl_client_socket_nss.
diff --git a/net/base/client_socket_pool.cc b/net/base/client_socket_pool.cc
index b8434dd..41c1b7f 100644
--- a/net/base/client_socket_pool.cc
+++ b/net/base/client_socket_pool.cc
@@ -13,11 +13,12 @@ using base::TimeDelta;
namespace {
-// The timeout value, in seconds, used to clean up disconnected idle sockets.
+// The timeout value, in seconds, used to clean up idle sockets that can't be
+// reused.
const int kCleanupInterval = 10;
// The maximum duration, in seconds, to keep idle persistent sockets alive.
-const int kIdleTimeout = 300; // 5 minutes.
+const int kIdleTimeout = 300; // 5 minutes.
} // namespace
@@ -54,12 +55,12 @@ int ClientSocketPool::RequestSocket(ClientSocketHandle* handle,
group.active_socket_count++;
// Use idle sockets in LIFO order because they're more likely to be
- // still connected.
+ // still reusable.
while (!group.idle_sockets.empty()) {
IdleSocket idle_socket = group.idle_sockets.back();
group.idle_sockets.pop_back();
DecrementIdleCount();
- if ((*idle_socket.ptr)->IsConnected()) {
+ if ((*idle_socket.ptr)->IsConnectedAndIdle()) {
// We found one we can reuse!
handle->socket_ = idle_socket.ptr;
return OK;
@@ -103,9 +104,9 @@ void ClientSocketPool::CloseIdleSockets() {
}
bool ClientSocketPool::IdleSocket::ShouldCleanup(base::TimeTicks now) const {
- bool timed_out = (now - start_time) >=
+ bool timed_out = (now - start_time) >=
base::TimeDelta::FromSeconds(kIdleTimeout);
- return timed_out || !(*ptr)->IsConnected();
+ return timed_out || !(*ptr)->IsConnectedAndIdle();
}
void ClientSocketPool::CleanupIdleSockets(bool force) {
@@ -162,7 +163,7 @@ void ClientSocketPool::DoReleaseSocket(const std::string& group_name,
DCHECK(group.active_socket_count > 0);
group.active_socket_count--;
- bool can_reuse = ptr->get() && (*ptr)->IsConnected();
+ bool can_reuse = ptr->get() && (*ptr)->IsConnectedAndIdle();
if (can_reuse) {
IdleSocket idle_socket;
idle_socket.ptr = ptr;
diff --git a/net/base/client_socket_pool.h b/net/base/client_socket_pool.h
index 764f859..cb43637 100644
--- a/net/base/client_socket_pool.h
+++ b/net/base/client_socket_pool.h
@@ -77,7 +77,7 @@ class ClientSocketPool : public base::RefCounted<ClientSocketPool> {
~ClientSocketPool();
// Closes all idle sockets if |force| is true. Else, only closes idle
- // sockets that are disconnected or timed out.
+ // sockets that timed out or can't be reused.
void CleanupIdleSockets(bool force);
// Called when the number of idle sockets changes.
@@ -88,7 +88,7 @@ class ClientSocketPool : public base::RefCounted<ClientSocketPool> {
void DoReleaseSocket(const std::string& group_name, ClientSocketPtr* ptr);
// Called when timer_ fires. This method scans the idle sockets removing
- // sockets that are disconnected or timed out.
+ // sockets that timed out or can't be reused.
void OnCleanupTimerFired() {
CleanupIdleSockets(false);
}
@@ -105,8 +105,13 @@ class ClientSocketPool : public base::RefCounted<ClientSocketPool> {
ClientSocketPtr* ptr;
base::TimeTicks start_time;
- // An idle socket should be removed if it is disconnected, or has been idle
+ // An idle socket should be removed if it can't be reused, or has been idle
// for too long. |now| is the current time value (TimeTicks::Now()).
+ //
+ // An idle socket can't be reused if it is disconnected or has received
+ // data unexpectedly (hence no longer idle). The unread data would be
+ // mistaken for the beginning of the next response if we were to reuse the
+ // socket for a new request.
bool ShouldCleanup(base::TimeTicks now) const;
};
@@ -122,8 +127,8 @@ class ClientSocketPool : public base::RefCounted<ClientSocketPool> {
typedef std::map<std::string, Group> GroupMap;
GroupMap group_map_;
- // Timer used to periodically prune idle sockets that are disconnected or
- // timed out.
+ // Timer used to periodically prune idle sockets that timed out or can't be
+ // reused.
base::RepeatingTimer<ClientSocketPool> timer_;
// The total number of idle sockets in the system.
@@ -131,7 +136,7 @@ class ClientSocketPool : public base::RefCounted<ClientSocketPool> {
// The maximum number of sockets kept per group.
int max_sockets_per_group_;
-
+
DISALLOW_COPY_AND_ASSIGN(ClientSocketPool);
};
diff --git a/net/base/client_socket_pool_unittest.cc b/net/base/client_socket_pool_unittest.cc
index affae15..5b0836b 100644
--- a/net/base/client_socket_pool_unittest.cc
+++ b/net/base/client_socket_pool_unittest.cc
@@ -35,6 +35,9 @@ class MockClientSocket : public net::ClientSocket {
virtual bool IsConnected() const {
return connected_;
}
+ virtual bool IsConnectedAndIdle() const {
+ return connected_;
+ }
// Socket methods:
virtual int Read(char* buf, int buf_len,
@@ -56,7 +59,7 @@ int MockClientSocket::allocation_count = 0;
class TestSocketRequest : public CallbackRunner< Tuple1<int> > {
public:
- TestSocketRequest(net::ClientSocketPool* pool) : handle(pool) {}
+ explicit TestSocketRequest(net::ClientSocketPool* pool) : handle(pool) {}
net::ClientSocketHandle handle;
diff --git a/net/base/ssl_client_socket_mac.cc b/net/base/ssl_client_socket_mac.cc
index a84929b..d351db5 100644
--- a/net/base/ssl_client_socket_mac.cc
+++ b/net/base/ssl_client_socket_mac.cc
@@ -307,6 +307,17 @@ bool SSLClientSocketMac::IsConnected() const {
return completed_handshake_ && transport_->IsConnected();
}
+bool SSLClientSocketMac::IsConnectedAndIdle() const {
+ // Unlike IsConnected, this method doesn't return a false positive.
+ //
+ // Strictly speaking, we should check if we have received the close_notify
+ // alert message from the server, and return false in that case. Although
+ // the close_notify alert message means EOF in the SSL layer, it is just
+ // bytes to the transport layer below, so transport_->IsConnectedAndIdle()
+ // returns the desired false when we receive close_notify.
+ return completed_handshake_ && transport_->IsConnectedAndIdle();
+}
+
int SSLClientSocketMac::Read(char* buf, int buf_len,
CompletionCallback* callback) {
DCHECK(completed_handshake_);
diff --git a/net/base/ssl_client_socket_mac.h b/net/base/ssl_client_socket_mac.h
index 1c74b68..c8a43ce 100644
--- a/net/base/ssl_client_socket_mac.h
+++ b/net/base/ssl_client_socket_mac.h
@@ -37,6 +37,7 @@ class SSLClientSocketMac : public SSLClientSocket {
virtual int ReconnectIgnoringLastError(CompletionCallback* callback);
virtual void Disconnect();
virtual bool IsConnected() const;
+ virtual bool IsConnectedAndIdle() const;
// Socket methods:
virtual int Read(char* buf, int buf_len, CompletionCallback* callback);
diff --git a/net/base/ssl_client_socket_nss.cc b/net/base/ssl_client_socket_nss.cc
index f67c246..648f807 100644
--- a/net/base/ssl_client_socket_nss.cc
+++ b/net/base/ssl_client_socket_nss.cc
@@ -165,12 +165,32 @@ void SSLClientSocketNSS::Disconnect() {
}
bool SSLClientSocketNSS::IsConnected() const {
+ // Ideally, we should also check if we have received the close_notify alert
+ // message from the server, and return false in that case. We're not doing
+ // that, so this function may return a false positive. Since the upper
+ // layer (HttpNetworkTransaction) needs to handle a persistent connection
+ // closed by the server when we send a request anyway, a false positive in
+ // exchange for simpler code is a good trade-off.
EnterFunction("");
bool ret = completed_handshake_ && transport_->IsConnected();
LeaveFunction("");
return ret;
}
+bool SSLClientSocketNSS::IsConnectedAndIdle() const {
+ // Unlike IsConnected, this method doesn't return a false positive.
+ //
+ // Strictly speaking, we should check if we have received the close_notify
+ // alert message from the server, and return false in that case. Although
+ // the close_notify alert message means EOF in the SSL layer, it is just
+ // bytes to the transport layer below, so transport_->IsConnectedAndIdle()
+ // returns the desired false when we receive close_notify.
+ EnterFunction("");
+ bool ret = completed_handshake_ && transport_->IsConnectedAndIdle();
+ LeaveFunction("");
+ return ret;
+}
+
int SSLClientSocketNSS::Read(char* buf, int buf_len,
CompletionCallback* callback) {
EnterFunction(buf_len);
diff --git a/net/base/ssl_client_socket_nss.h b/net/base/ssl_client_socket_nss.h
index b16557c..6729598 100644
--- a/net/base/ssl_client_socket_nss.h
+++ b/net/base/ssl_client_socket_nss.h
@@ -37,6 +37,7 @@ class SSLClientSocketNSS : public SSLClientSocket {
virtual int ReconnectIgnoringLastError(CompletionCallback* callback);
virtual void Disconnect();
virtual bool IsConnected() const;
+ virtual bool IsConnectedAndIdle() const;
// Socket methods:
virtual int Read(char* buf, int buf_len, CompletionCallback* callback);
diff --git a/net/base/ssl_client_socket_unittest.cc b/net/base/ssl_client_socket_unittest.cc
index 4710151..3264c36 100644
--- a/net/base/ssl_client_socket_unittest.cc
+++ b/net/base/ssl_client_socket_unittest.cc
@@ -166,6 +166,11 @@ TEST_F(SSLClientSocketTest, MAYBE_ConnectMismatched) {
EXPECT_TRUE(sock->IsConnected());
}
+// TODO(wtc): Add unit tests for IsConnectedAndIdle:
+// - Server closes an SSL connection (with a close_notify alert message).
+// - Server closes the underlying TCP connection directly.
+// - Server sends data unexpectedly.
+
TEST_F(SSLClientSocketTest, MAYBE_Read) {
StartOKServer();
diff --git a/net/base/ssl_client_socket_win.cc b/net/base/ssl_client_socket_win.cc
index 5f73746..6aba9c2 100644
--- a/net/base/ssl_client_socket_win.cc
+++ b/net/base/ssl_client_socket_win.cc
@@ -314,6 +314,17 @@ bool SSLClientSocketWin::IsConnected() const {
return completed_handshake_ && transport_->IsConnected();
}
+bool SSLClientSocketWin::IsConnectedAndIdle() const {
+ // Unlike IsConnected, this method doesn't return a false positive.
+ //
+ // Strictly speaking, we should check if we have received the close_notify
+ // alert message from the server, and return false in that case. Although
+ // the close_notify alert message means EOF in the SSL layer, it is just
+ // bytes to the transport layer below, so transport_->IsConnectedAndIdle()
+ // returns the desired false when we receive close_notify.
+ return completed_handshake_ && transport_->IsConnectedAndIdle();
+}
+
int SSLClientSocketWin::Read(char* buf, int buf_len,
CompletionCallback* callback) {
DCHECK(completed_handshake_);
diff --git a/net/base/ssl_client_socket_win.h b/net/base/ssl_client_socket_win.h
index 6fd19a0..feba718 100644
--- a/net/base/ssl_client_socket_win.h
+++ b/net/base/ssl_client_socket_win.h
@@ -42,6 +42,7 @@ class SSLClientSocketWin : public SSLClientSocket {
virtual int ReconnectIgnoringLastError(CompletionCallback* callback);
virtual void Disconnect();
virtual bool IsConnected() const;
+ virtual bool IsConnectedAndIdle() const;
// Socket methods:
virtual int Read(char* buf, int buf_len, CompletionCallback* callback);
diff --git a/net/base/tcp_client_socket.h b/net/base/tcp_client_socket.h
index e4a0848..abb3b8e 100644
--- a/net/base/tcp_client_socket.h
+++ b/net/base/tcp_client_socket.h
@@ -50,6 +50,7 @@ class TCPClientSocket : public ClientSocket,
virtual int ReconnectIgnoringLastError(CompletionCallback* callback);
virtual void Disconnect();
virtual bool IsConnected() const;
+ virtual bool IsConnectedAndIdle() const;
// Socket methods:
// Multiple outstanding requests are not supported.
diff --git a/net/base/tcp_client_socket_libevent.cc b/net/base/tcp_client_socket_libevent.cc
index 7a5776a..4722094 100644
--- a/net/base/tcp_client_socket_libevent.cc
+++ b/net/base/tcp_client_socket_libevent.cc
@@ -161,6 +161,22 @@ bool TCPClientSocket::IsConnected() const {
return true;
}
+bool TCPClientSocket::IsConnectedAndIdle() const {
+ if (socket_ == kInvalidSocket || waiting_connect_)
+ return false;
+
+ // Check if connection is alive and we haven't received any data
+ // unexpectedly.
+ char c;
+ int rv = recv(socket_, &c, 1, MSG_PEEK);
+ if (rv >= 0)
+ return false;
+ if (errno != EAGAIN && errno != EWOULDBLOCK)
+ return false;
+
+ return true;
+}
+
int TCPClientSocket::Read(char* buf,
int buf_len,
CompletionCallback* callback) {
diff --git a/net/base/tcp_client_socket_unittest.cc b/net/base/tcp_client_socket_unittest.cc
index 976ff1e..536528b 100644
--- a/net/base/tcp_client_socket_unittest.cc
+++ b/net/base/tcp_client_socket_unittest.cc
@@ -89,6 +89,10 @@ TEST_F(TCPClientSocketTest, Connect) {
EXPECT_FALSE(sock.IsConnected());
}
+// TODO(wtc): Add unit tests for IsConnectedAndIdle:
+// - Server closes a connection.
+// - Server sends data unexpectedly.
+
TEST_F(TCPClientSocketTest, Read) {
net::AddressList addr;
net::HostResolver resolver;
diff --git a/net/base/tcp_client_socket_win.cc b/net/base/tcp_client_socket_win.cc
index d2a1452..18afe03c 100644
--- a/net/base/tcp_client_socket_win.cc
+++ b/net/base/tcp_client_socket_win.cc
@@ -157,6 +157,22 @@ bool TCPClientSocket::IsConnected() const {
return true;
}
+bool TCPClientSocket::IsConnectedAndIdle() const {
+ if (socket_ == INVALID_SOCKET || wait_state_ == WAITING_CONNECT)
+ return false;
+
+ // Check if connection is alive and we haven't received any data
+ // unexpectedly.
+ char c;
+ int rv = recv(socket_, &c, 1, MSG_PEEK);
+ if (rv >= 0)
+ return false;
+ if (WSAGetLastError() != WSAEWOULDBLOCK)
+ return false;
+
+ return true;
+}
+
int TCPClientSocket::Read(char* buf,
int buf_len,
CompletionCallback* callback) {
diff --git a/net/http/http_network_transaction_unittest.cc b/net/http/http_network_transaction_unittest.cc
index 52e299f..5ceeb39 100644
--- a/net/http/http_network_transaction_unittest.cc
+++ b/net/http/http_network_transaction_unittest.cc
@@ -109,6 +109,9 @@ class MockTCPClientSocket : public net::ClientSocket {
virtual bool IsConnected() const {
return connected_;
}
+ virtual bool IsConnectedAndIdle() const {
+ return connected_;
+ }
// Socket methods:
virtual int Read(char* buf, int buf_len, net::CompletionCallback* callback) {
DCHECK(!callback_);
generated by cgit v1.1 at 2025-02-01 08:54:57 +0000