diff options
author | dalecurtis <dalecurtis@chromium.org> | 2014-09-15 17:41:22 -0700 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2014-09-16 00:44:25 +0000 |
commit | 3bdbb01600f8ca78609794c23f2f1c8410a78e98 (patch) | |
tree | ac764ba1e651efdcd7b28174954be81cddb63f75 /base | |
parent | b2f930d5f00340efae6a517e437b6057b47f6a84 (diff) | |
download | chromium_src-3bdbb01600f8ca78609794c23f2f1c8410a78e98.zip chromium_src-3bdbb01600f8ca78609794c23f2f1c8410a78e98.tar.gz chromium_src-3bdbb01600f8ca78609794c23f2f1c8410a78e98.tar.bz2 |
Avoid unsafe usage of CancelIo() within Windows SyncSocket.
The CancelIo() operation does not synchronously cancel IO operations,
we must wait for them to return before returning to avoid writes to
resources which may have already been destructed.
See this blog post for more information:
http://blogs.msdn.com/b/oldnewthing/archive/2011/02/02/10123392.aspx
BUG=413494
TEST=manual inspect of cancellation states.
Review URL: https://codereview.chromium.org/570713003
Cr-Commit-Position: refs/heads/master@{#294946}
Diffstat (limited to 'base')
-rw-r--r-- | base/sync_socket_win.cc | 30 |
1 files changed, 18 insertions, 12 deletions
diff --git a/base/sync_socket_win.cc b/base/sync_socket_win.cc index 3c40eba..6b2575c 100644 --- a/base/sync_socket_win.cc +++ b/base/sync_socket_win.cc @@ -154,21 +154,27 @@ size_t CancelableFileOperation(Function operation, timeout_in_ms == INFINITE ? timeout_in_ms : (finish_time - current_time).InMilliseconds()); - if (wait_result == (WAIT_OBJECT_0 + 0)) { - GetOverlappedResult(file, &ol, &len, TRUE); - } else if (wait_result == (WAIT_OBJECT_0 + 1)) { - DVLOG(1) << "Shutdown was signaled. Closing socket."; + if (wait_result != WAIT_OBJECT_0 + 0) { + // CancelIo() doesn't synchronously cancel outstanding IO, only marks + // outstanding IO for cancellation. We must call GetOverlappedResult() + // below to ensure in flight writes complete before returning. CancelIo(file); + } + + // We set the |bWait| parameter to TRUE for GetOverlappedResult() to + // ensure writes are complete before returning. + if (!GetOverlappedResult(file, &ol, &len, TRUE)) + len = 0; + + if (wait_result == WAIT_OBJECT_0 + 1) { + DVLOG(1) << "Shutdown was signaled. Closing socket."; socket->Close(); - count = 0; - break; - } else { - // Timeout happened. - DCHECK_EQ(WAIT_TIMEOUT, wait_result); - if (!CancelIo(file)) - DLOG(WARNING) << "CancelIo() failed"; - break; + return count; } + + // Timeouts will be handled by the while() condition below since + // GetOverlappedResult() may complete successfully after CancelIo(). + DCHECK(wait_result == WAIT_OBJECT_0 + 0 || wait_result == WAIT_TIMEOUT); } else { break; } |