Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(394)

Unified Diff: net/socket/tcp_client_socket_win.cc

Issue 12468002: Report the correct os_error in the SOCKET_READ_ERROR event (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src/
Patch Set: Remove a potentially misleading comment. Created 7 years, 9 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « net/socket/ssl_client_socket_nss.cc ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: net/socket/tcp_client_socket_win.cc
===================================================================
--- net/socket/tcp_client_socket_win.cc (revision 187920)
+++ net/socket/tcp_client_socket_win.cc (working copy)
@@ -957,7 +957,6 @@
DWORD num_bytes, flags;
BOOL ok = WSAGetOverlappedResult(socket_, &core_->read_overlapped_,
&num_bytes, FALSE, &flags);
- WSAResetEvent(core_->read_overlapped_.hEvent);
waiting_read_ = false;
int rv;
if (ok) {
@@ -975,6 +974,7 @@
net_log_.AddEvent(NetLog::TYPE_SOCKET_READ_ERROR,
CreateNetLogSocketErrorCallback(rv, os_error));
}
+ WSAResetEvent(core_->read_overlapped_.hEvent);
core_->read_iobuffer_ = NULL;
core_->read_buffer_length_ = 0;
DoReadCallback(rv);
@@ -1025,22 +1025,29 @@
if (rv == SOCKET_ERROR) {
os_error = WSAGetLastError();
rv = MapSystemError(os_error);
- } else if (network_events.lNetworkEvents & FD_READ) {
+ } else if (network_events.lNetworkEvents) {
+ DCHECK_EQ(network_events.lNetworkEvents & ~(FD_READ | FD_CLOSE), 0);
+ // If network_events.lNetworkEvents is FD_CLOSE and
+ // network_events.iErrorCode[FD_CLOSE_BIT] is 0, it is a graceful
+ // connection closure. It is tempting to directly set rv to 0 in
+ // this case, but the MSDN pages for WSAEventSelect and
+ // WSAAsyncSelect recommend we still call DoRead():
+ // FD_CLOSE should only be posted after all data is read from a
+ // socket, but an application should check for remaining data upon
+ // receipt of FD_CLOSE to avoid any possibility of losing data.
+ //
+ // If network_events.iErrorCode[FD_READ_BIT] or
+ // network_events.iErrorCode[FD_CLOSE_BIT] is nonzero, still call
+ // DoRead() because recv() reports a more accurate error code
+ // (WSAECONNRESET vs. WSAECONNABORTED) when the connection was
+ // reset.
rv = DoRead(core_->read_iobuffer_, core_->read_buffer_length_,
read_callback_);
if (rv == ERR_IO_PENDING)
return;
- } else if (network_events.lNetworkEvents & FD_CLOSE) {
- if (network_events.iErrorCode[FD_CLOSE_BIT]) {
- rv = MapSystemError(network_events.iErrorCode[FD_CLOSE_BIT]);
- net_log_.AddEvent(NetLog::TYPE_SOCKET_READ_ERROR,
- CreateNetLogSocketErrorCallback(rv, os_error));
- } else {
- rv = 0;
- }
} else {
- // This should not happen but I have seen cases where we will get
- // signaled but the network events flags are all clear (0).
+ // This may happen because Read() may succeed synchronously and
+ // consume all the received data without resetting the event object.
core_->WatchForRead();
return;
}
« no previous file with comments | « net/socket/ssl_client_socket_nss.cc ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698