Commit 3bdbb016 authored by dalecurtis's avatar dalecurtis Committed by Commit bot

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}
parent b2f930d5
...@@ -154,21 +154,27 @@ size_t CancelableFileOperation(Function operation, ...@@ -154,21 +154,27 @@ size_t CancelableFileOperation(Function operation,
timeout_in_ms == INFINITE timeout_in_ms == INFINITE
? timeout_in_ms ? timeout_in_ms
: (finish_time - current_time).InMilliseconds()); : (finish_time - current_time).InMilliseconds());
if (wait_result == (WAIT_OBJECT_0 + 0)) { if (wait_result != WAIT_OBJECT_0 + 0) {
GetOverlappedResult(file, &ol, &len, TRUE); // CancelIo() doesn't synchronously cancel outstanding IO, only marks
} else if (wait_result == (WAIT_OBJECT_0 + 1)) { // outstanding IO for cancellation. We must call GetOverlappedResult()
DVLOG(1) << "Shutdown was signaled. Closing socket."; // below to ensure in flight writes complete before returning.
CancelIo(file); 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(); socket->Close();
count = 0; return count;
break;
} else {
// Timeout happened.
DCHECK_EQ(WAIT_TIMEOUT, wait_result);
if (!CancelIo(file))
DLOG(WARNING) << "CancelIo() failed";
break;
} }
// 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 { } else {
break; break;
} }
......
...@@ -144,10 +144,11 @@ bool AudioSyncReader::WaitUntilDataIsReady() { ...@@ -144,10 +144,11 @@ bool AudioSyncReader::WaitUntilDataIsReady() {
while (timeout.InMicroseconds() > 0) { while (timeout.InMicroseconds() > 0) {
bytes_received = socket_->ReceiveWithTimeout( bytes_received = socket_->ReceiveWithTimeout(
&renderer_buffer_index, sizeof(renderer_buffer_index), timeout); &renderer_buffer_index, sizeof(renderer_buffer_index), timeout);
if (!bytes_received) if (bytes_received != sizeof(renderer_buffer_index)) {
bytes_received = 0;
break; break;
}
DCHECK_EQ(bytes_received, sizeof(renderer_buffer_index));
if (renderer_buffer_index == buffer_index_) if (renderer_buffer_index == buffer_index_)
break; break;
......
...@@ -166,10 +166,8 @@ void AudioDeviceThread::Thread::Run() { ...@@ -166,10 +166,8 @@ void AudioDeviceThread::Thread::Run() {
while (true) { while (true) {
int pending_data = 0; int pending_data = 0;
size_t bytes_read = socket_.Receive(&pending_data, sizeof(pending_data)); size_t bytes_read = socket_.Receive(&pending_data, sizeof(pending_data));
if (bytes_read != sizeof(pending_data)) { if (bytes_read != sizeof(pending_data))
DCHECK_EQ(bytes_read, 0U);
break; break;
}
{ {
base::AutoLock auto_lock(callback_lock_); base::AutoLock auto_lock(callback_lock_);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment