Commit 48f85308 authored by Gabriel Charette's avatar Gabriel Charette Committed by Commit Bot

[PlatformThread] Mark PlatformThread::Join as blocking on Windows as well.

Aligning with the POSIX impl in preparation for
https://chromium-review.googlesource.com/c/chromium/src/+/1283010

Also adds two trivial ScopedAllowBaseSyncPrimitivesForTesting around
existing thread joins in tests (more advanced changes were extracted
to precursor CLs).

Bug: 707362
Change-Id: If4c4c49b4a20554518ca4a25ae1ea16b84d3a510
Reviewed-on: https://chromium-review.googlesource.com/c/1324370
Commit-Queue: Gabriel Charette <gab@chromium.org>
Reviewed-by: default avatarBret Sepulveda <bsep@chromium.org>
Reviewed-by: default avatarkylechar <kylechar@chromium.org>
Reviewed-by: default avatarRyan Hamilton <rch@chromium.org>
Cr-Commit-Position: refs/heads/master@{#609013}
parent f4d5827f
...@@ -264,6 +264,8 @@ void PlatformThread::Join(PlatformThreadHandle thread_handle) { ...@@ -264,6 +264,8 @@ void PlatformThread::Join(PlatformThreadHandle thread_handle) {
// Joining another thread may block the current thread for a long time, since // Joining another thread may block the current thread for a long time, since
// the thread referred to by |thread_handle| may still be running long-lived / // the thread referred to by |thread_handle| may still be running long-lived /
// blocking tasks. // blocking tasks.
// TODO(https://crbug.com/707362): Make this a
// ScopedBlockingCallWithBaseSyncPrimitives.
base::ScopedBlockingCall scoped_blocking_call(base::BlockingType::MAY_BLOCK); base::ScopedBlockingCall scoped_blocking_call(base::BlockingType::MAY_BLOCK);
CHECK_EQ(0, pthread_join(thread_handle.platform_handle(), nullptr)); CHECK_EQ(0, pthread_join(thread_handle.platform_handle(), nullptr));
} }
......
...@@ -12,6 +12,7 @@ ...@@ -12,6 +12,7 @@
#include "base/logging.h" #include "base/logging.h"
#include "base/metrics/histogram_macros.h" #include "base/metrics/histogram_macros.h"
#include "base/strings/utf_string_conversions.h" #include "base/strings/utf_string_conversions.h"
#include "base/threading/scoped_blocking_call.h"
#include "base/threading/thread_id_name_manager.h" #include "base/threading/thread_id_name_manager.h"
#include "base/threading/thread_restrictions.h" #include "base/threading/thread_restrictions.h"
#include "base/win/scoped_handle.h" #include "base/win/scoped_handle.h"
...@@ -259,12 +260,6 @@ bool PlatformThread::CreateNonJoinableWithPriority(size_t stack_size, ...@@ -259,12 +260,6 @@ bool PlatformThread::CreateNonJoinableWithPriority(size_t stack_size,
// static // static
void PlatformThread::Join(PlatformThreadHandle thread_handle) { void PlatformThread::Join(PlatformThreadHandle thread_handle) {
DCHECK(thread_handle.platform_handle()); DCHECK(thread_handle.platform_handle());
// TODO(willchan): Enable this check once I can get it to work for Windows
// shutdown.
// Joining another thread may block the current thread for a long time, since
// the thread referred to by |thread_handle| may still be running long-lived /
// blocking tasks.
// AssertBlockingAllowedDeprecated();
DWORD thread_id = 0; DWORD thread_id = 0;
thread_id = ::GetThreadId(thread_handle.platform_handle()); thread_id = ::GetThreadId(thread_handle.platform_handle());
...@@ -279,6 +274,10 @@ void PlatformThread::Join(PlatformThreadHandle thread_handle) { ...@@ -279,6 +274,10 @@ void PlatformThread::Join(PlatformThreadHandle thread_handle) {
// Record the event that this thread is blocking upon (for hang diagnosis). // Record the event that this thread is blocking upon (for hang diagnosis).
base::debug::ScopedThreadJoinActivity thread_activity(&thread_handle); base::debug::ScopedThreadJoinActivity thread_activity(&thread_handle);
// TODO(https://crbug.com/707362): Make this a
// ScopedBlockingCallWithBaseSyncPrimitives.
base::ScopedBlockingCall scoped_blocking_call(base::BlockingType::MAY_BLOCK);
// Wait for the thread to exit. It should already have terminated but make // Wait for the thread to exit. It should already have terminated but make
// sure this assumption is valid. // sure this assumption is valid.
CHECK_EQ(WAIT_OBJECT_0, CHECK_EQ(WAIT_OBJECT_0,
......
...@@ -11,6 +11,7 @@ ...@@ -11,6 +11,7 @@
#include "base/run_loop.h" #include "base/run_loop.h"
#include "base/single_thread_task_runner.h" #include "base/single_thread_task_runner.h"
#include "base/strings/utf_string_conversions.h" #include "base/strings/utf_string_conversions.h"
#include "base/threading/thread_restrictions.h"
#include "build/build_config.h" #include "build/build_config.h"
#include "chrome/app/chrome_command_ids.h" #include "chrome/app/chrome_command_ids.h"
#include "chrome/browser/bookmarks/bookmark_model_factory.h" #include "chrome/browser/bookmarks/bookmark_model_factory.h"
...@@ -124,6 +125,7 @@ void ToolbarViewInteractiveUITest::TestWhileInDragOperation() { ...@@ -124,6 +125,7 @@ void ToolbarViewInteractiveUITest::TestWhileInDragOperation() {
void ToolbarViewInteractiveUITest::FinishDragAndDrop( void ToolbarViewInteractiveUITest::FinishDragAndDrop(
base::Closure quit_closure) { base::Closure quit_closure) {
base::ScopedAllowBlockingForTesting allow_thread_join;
dnd_thread_.reset(); dnd_thread_.reset();
TestWhileInDragOperation(); TestWhileInDragOperation();
ui_controls::SendMouseEventsNotifyWhenDone(ui_controls::LEFT, ui_controls::UP, ui_controls::SendMouseEventsNotifyWhenDone(ui_controls::LEFT, ui_controls::UP,
......
...@@ -19,6 +19,7 @@ ...@@ -19,6 +19,7 @@
#include "base/strings/utf_string_conversions.h" #include "base/strings/utf_string_conversions.h"
#include "base/test/test_timeouts.h" #include "base/test/test_timeouts.h"
#include "base/threading/thread.h" #include "base/threading/thread.h"
#include "base/threading/thread_restrictions.h"
#include "base/win/scoped_handle.h" #include "base/win/scoped_handle.h"
#include "net/test/python_utils.h" #include "net/test/python_utils.h"
...@@ -71,7 +72,9 @@ bool ReadData(HANDLE read_fd, ...@@ -71,7 +72,9 @@ bool ReadData(HANDLE read_fd,
bytes_read += num_bytes; bytes_read += num_bytes;
} }
base::ScopedAllowBlockingForTesting allow_thread_join;
thread.Stop(); thread.Stop();
// If the timeout kicked in, abort. // If the timeout kicked in, abort.
if (unblocked) { if (unblocked) {
LOG(ERROR) << "Timeout exceeded for ReadData"; LOG(ERROR) << "Timeout exceeded for ReadData";
......
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