diff options
author | Allan Sandfeld Jensen <allan.jensen@qt.io> | 2021-10-26 13:57:00 +0200 |
---|---|---|
committer | Allan Sandfeld Jensen <allan.jensen@qt.io> | 2021-11-02 11:31:01 +0000 |
commit | 1943b3c2a1dcee36c233724fc4ee7613d71b9cf6 (patch) | |
tree | 8c1b5f12357025c197da5427ae02cfdc2f3570d6 /chromium/base/threading | |
parent | 21ba0c5d4bf8fba15dddd97cd693bad2358b77fd (diff) |
BASELINE: Update Chromium to 94.0.4606.111
Change-Id: I924781584def20fc800bedf6ff41fdb96c438193
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>
Diffstat (limited to 'chromium/base/threading')
22 files changed, 451 insertions, 399 deletions
diff --git a/chromium/base/threading/hang_watcher.cc b/chromium/base/threading/hang_watcher.cc index 1a3d87a9d03..7e8bfcb8586 100644 --- a/chromium/base/threading/hang_watcher.cc +++ b/chromium/base/threading/hang_watcher.cc @@ -24,6 +24,7 @@ #include "base/threading/platform_thread.h" #include "base/threading/thread_checker.h" #include "base/threading/thread_restrictions.h" +#include "base/threading/threading_features.h" #include "base/time/default_tick_clock.h" #include "base/time/time.h" #include "build/build_config.h" @@ -31,33 +32,13 @@ namespace base { namespace { + // Defines how much logging happens when the HangWatcher monitors the threads. // Logging levels are set per thread type through Finch. It's important that // the order of the enum members stay the and that their numerical // values be in increasing order. The implementation of // ThreadTypeLoggingLevelGreaterOrEqual() depends on it. enum class LoggingLevel { kNone = 0, kUmaOnly = 1, kUmaAndCrash = 2 }; -} // namespace - -// static -const base::Feature HangWatcher::kEnableHangWatcher{ - "EnableHangWatcher", base::FEATURE_ENABLED_BY_DEFAULT}; - -constexpr base::FeatureParam<int> kIOThreadLogLevel{ - &HangWatcher::kEnableHangWatcher, "io_thread_log_level", - static_cast<int>(LoggingLevel::kUmaOnly)}; -constexpr base::FeatureParam<int> kUIThreadLogLevel{ - &HangWatcher::kEnableHangWatcher, "ui_thread_log_level", - static_cast<int>(LoggingLevel::kUmaOnly)}; -constexpr base::FeatureParam<int> kThreadPoolLogLevel{ - &HangWatcher::kEnableHangWatcher, "threadpool_log_level", - static_cast<int>(LoggingLevel::kNone)}; - -// static -const base::TimeDelta WatchHangsInScope::kDefaultHangWatchTime = - base::TimeDelta::FromSeconds(10); - -namespace { HangWatcher* g_instance = nullptr; std::atomic<bool> g_use_hang_watcher{false}; @@ -110,7 +91,27 @@ bool ThreadTypeLoggingLevelGreaterOrEqual(HangWatcher::ThreadType thread_type, logging_level; } } -} + +} // namespace + +// Determines if the HangWatcher is activated. When false the HangWatcher +// thread never started. +const Feature kEnableHangWatcher{"EnableHangWatcher", + FEATURE_ENABLED_BY_DEFAULT}; + +constexpr base::FeatureParam<int> kIOThreadLogLevel{ + &kEnableHangWatcher, "io_thread_log_level", + static_cast<int>(LoggingLevel::kUmaOnly)}; +constexpr base::FeatureParam<int> kUIThreadLogLevel{ + &kEnableHangWatcher, "ui_thread_log_level", + static_cast<int>(LoggingLevel::kUmaOnly)}; +constexpr base::FeatureParam<int> kThreadPoolLogLevel{ + &kEnableHangWatcher, "threadpool_log_level", + static_cast<int>(LoggingLevel::kUmaOnly)}; + +// static +const base::TimeDelta WatchHangsInScope::kDefaultHangWatchTime = + base::TimeDelta::FromSeconds(10); constexpr const char* kThreadName = "HangWatcher"; @@ -136,10 +137,6 @@ WatchHangsInScope::WatchHangsInScope(TimeDelta timeout) { return; } - DCHECK(current_hang_watch_state) - << "WatchHangsInScope can only be used on a thread that " - "registered for hang watching with HangWatcher::RegisterThread."; - #if DCHECK_IS_ON() previous_watch_hangs_in_scope_ = current_hang_watch_state->GetCurrentWatchHangsInScope(); @@ -151,25 +148,6 @@ WatchHangsInScope::WatchHangsInScope(TimeDelta timeout) { std::tie(old_flags, old_deadline) = current_hang_watch_state->GetFlagsAndDeadline(); - const bool hangs_ignored_for_current_scope = - internal::HangWatchDeadline::IsFlagSet( - internal::HangWatchDeadline::Flag::kIgnoreCurrentWatchHangsInScope, - old_flags); - - const bool has_active_hang_watch_disabled = - internal::HangWatchDeadline::IsFlagSet( - internal::HangWatchDeadline::Flag::kHasActiveIgnoreHangsInScope, - old_flags); - - // If the current WatchHangsInScope is ignored but there are no active - // IgnoreHangsInScope instances, temporarily reactivate hang watching for - // this newly created WatchHangsInScope. On exiting hang watching is - // suspended again to return to the original state. - if (hangs_ignored_for_current_scope && !has_active_hang_watch_disabled) { - current_hang_watch_state->UnsetIgnoreCurrentWatchHangsInScope(); - set_hangs_ignored_on_exit_ = true; - } - // TODO(crbug.com/1034046): Check whether we are over deadline already for the // previous WatchHangsInScope here by issuing only one TimeTicks::Now() // and resuing the value. @@ -178,6 +156,19 @@ WatchHangsInScope::WatchHangsInScope(TimeDelta timeout) { TimeTicks deadline = TimeTicks::Now() + timeout; current_hang_watch_state->SetDeadline(deadline); current_hang_watch_state->IncrementNestingLevel(); + + const bool hangs_ignored_for_current_scope = + internal::HangWatchDeadline::IsFlagSet( + internal::HangWatchDeadline::Flag::kIgnoreCurrentWatchHangsInScope, + old_flags); + + // If the current WatchHangsInScope is ignored, temporarily reactivate hang + // watching for newly created WatchHangsInScopes. On exiting hang watching + // is suspended again to return to the original state. + if (hangs_ignored_for_current_scope) { + current_hang_watch_state->UnsetIgnoreCurrentWatchHangsInScope(); + set_hangs_ignored_on_exit_ = true; + } } WatchHangsInScope::~WatchHangsInScope() { @@ -192,6 +183,12 @@ WatchHangsInScope::~WatchHangsInScope() { return; } + // If the thread was unregistered since construction there is also nothing to + // do . + if (!current_hang_watch_state) { + return; + } + // If a hang is currently being captured we should block here so execution // stops and we avoid recording unrelated stack frames in the crash. if (current_hang_watch_state->IsFlagSet( @@ -205,19 +202,18 @@ WatchHangsInScope::~WatchHangsInScope() { previous_watch_hangs_in_scope_); #endif - // If a IgnoreHangsInScope suspended hang watching during the - // lifetime of this or any nested WatchHangsInScope it can now safely be - // reactivated by clearing the ignore bit since this is the outer-most scope. - // See IgnoreHangsInScope class comments where this represents the - // destruction of |scope_1|. - if (current_hang_watch_state->nesting_level() == 1) + if (current_hang_watch_state->nesting_level() == 1) { + // If a call to InvalidateActiveExpectations() suspended hang watching + // during the lifetime of this or any nested WatchHangsInScope it can now + // safely be reactivated by clearing the ignore bit since this is the + // outer-most scope. current_hang_watch_state->UnsetIgnoreCurrentWatchHangsInScope(); - // Return to ignoring hangs since this was the previous state before hang - // watching was temporarily enabled for this WatchHangsInScope only in the - // constructor. See IgnoreHangsInScope class comments where the next line - // of code is part of the destruction of |scope_4|. - else if (set_hangs_ignored_on_exit_) + } else if (set_hangs_ignored_on_exit_) { + // Return to ignoring hangs since this was the previous state before hang + // watching was temporarily enabled for this WatchHangsInScope only in the + // constructor. current_hang_watch_state->SetIgnoreCurrentWatchHangsInScope(); + } // Reset the deadline to the value it had before entering this // WatchHangsInScope. @@ -228,49 +224,6 @@ WatchHangsInScope::~WatchHangsInScope() { current_hang_watch_state->DecrementNestingLevel(); } -IgnoreHangsInScope::IgnoreHangsInScope() { - internal::HangWatchState* current_hang_watch_state = - internal::HangWatchState::GetHangWatchStateForCurrentThread()->Get(); - if (!current_hang_watch_state) { - took_effect_ = false; - return; - } - - uint64_t old_flags; - base::TimeTicks old_deadline; - std::tie(old_flags, old_deadline) = - current_hang_watch_state->GetFlagsAndDeadline(); - - // If there already is an active IgnoreHangsInScope. - const bool has_active_hang_watch_disabled = - internal::HangWatchDeadline::IsFlagSet( - internal::HangWatchDeadline::Flag::kHasActiveIgnoreHangsInScope, - old_flags); - - if (has_active_hang_watch_disabled) { - took_effect_ = false; - return; - } - - current_hang_watch_state->SetIgnoreCurrentWatchHangsInScope(); - current_hang_watch_state->SetHasActiveIgnoreHangsInScope(); -} - -IgnoreHangsInScope::~IgnoreHangsInScope() { - internal::HangWatchState* current_hang_watch_state = - internal::HangWatchState::GetHangWatchStateForCurrentThread()->Get(); - - if (!current_hang_watch_state || !took_effect_) - return; - - // If this instance outlived all WatchHangsInScope instances watching - // needs to be reactivated. - if (current_hang_watch_state->nesting_level() == 0) - current_hang_watch_state->UnsetIgnoreCurrentWatchHangsInScope(); - - current_hang_watch_state->UnsetHasActiveIgnoreHangsInScope(); -} - // static void HangWatcher::InitializeOnMainThread() { DCHECK(!g_use_hang_watcher); @@ -321,12 +274,6 @@ bool HangWatcher::IsIOThreadHangWatchingEnabled() { } // static -bool HangWatcher::IsUIThreadHangWatchingEnabled() { - return g_ui_thread_log_level.load(std::memory_order_relaxed) != - LoggingLevel::kNone; -} - -// static bool HangWatcher::IsCrashReportingEnabled() { if (g_ui_thread_log_level.load(std::memory_order_relaxed) == LoggingLevel::kUmaAndCrash) { @@ -343,6 +290,17 @@ bool HangWatcher::IsCrashReportingEnabled() { return false; } +// static +void HangWatcher::InvalidateActiveExpectations() { + internal::HangWatchState* current_hang_watch_state = + internal::HangWatchState::GetHangWatchStateForCurrentThread()->Get(); + if (!current_hang_watch_state) { + // If the current thread is not under watch there is nothing to invalidate. + return; + } + current_hang_watch_state->SetIgnoreCurrentWatchHangsInScope(); +} + HangWatcher::HangWatcher() : monitor_period_(kMonitoringPeriod), should_monitor_(WaitableEvent::ResetPolicy::AUTOMATIC), @@ -602,8 +560,9 @@ HangWatcher::WatchStateSnapShot::WatchStateSnapShot( // the next capture then they'll already be marked and will be included // in the capture at that time. if (thread_marked && all_threads_marked) { - hung_watch_state_copies_.push_back( - WatchStateCopy{deadline, watch_state.get()->GetThreadID()}); + hung_watch_state_copies_.push_back(WatchStateCopy{ + deadline, + static_cast<PlatformThreadId>(watch_state.get()->GetThreadID())}); } else { all_threads_marked = false; } @@ -834,9 +793,7 @@ constexpr uint64_t kMaximumFlag = 0x8000000000000000u; constexpr uint64_t kPersistentFlagsAndDeadlineMask = kOnlyDeadlineMask | static_cast<uint64_t>( - HangWatchDeadline::Flag::kIgnoreCurrentWatchHangsInScope) | - static_cast<uint64_t>( - HangWatchDeadline::Flag::kHasActiveIgnoreHangsInScope); + HangWatchDeadline::Flag::kIgnoreCurrentWatchHangsInScope); } // namespace // Flag binary representation assertions. @@ -923,14 +880,6 @@ bool HangWatchDeadline::SetShouldBlockOnHang(uint64_t old_flags, std::memory_order_relaxed); } -void HangWatchDeadline::SetHasActiveIgnoreHangsInScope() { - SetPersistentFlag(Flag::kHasActiveIgnoreHangsInScope); -} - -void HangWatchDeadline::UnsetHasActiveIgnoreHangsInScope() { - ClearPersistentFlag(Flag::kHasActiveIgnoreHangsInScope); -} - void HangWatchDeadline::SetIgnoreCurrentWatchHangsInScope() { SetPersistentFlag(Flag::kIgnoreCurrentWatchHangsInScope); } @@ -999,10 +948,20 @@ uint64_t HangWatchDeadline::SwitchBitsForTesting() { } HangWatchState::HangWatchState(HangWatcher::ThreadType thread_type) - : thread_id_(PlatformThread::CurrentId()), thread_type_(thread_type) { + : thread_type_(thread_type) { // There should not exist a state object for this thread already. DCHECK(!GetHangWatchStateForCurrentThread()->Get()); +// TODO(crbug.com/1223033): Remove this once macOS uses system-wide ids. +// On macOS the thread ids used by CrashPad are not the same as the ones +// provided by PlatformThread. Make sure to use the same for correct +// attribution. +#ifdef OS_MAC + pthread_threadid_np(pthread_self(), &thread_id_); +#else + thread_id_ = PlatformThread::CurrentId(); +#endif + // Bind the new instance to this thread. GetHangWatchStateForCurrentThread()->Set(this); } @@ -1052,14 +1011,6 @@ bool HangWatchState::IsOverDeadline() const { return TimeTicks::Now() > deadline_.GetDeadline(); } -void HangWatchState::SetHasActiveIgnoreHangsInScope() { - deadline_.SetHasActiveIgnoreHangsInScope(); -} - -void HangWatchState::UnsetHasActiveIgnoreHangsInScope() { - deadline_.UnsetHasActiveIgnoreHangsInScope(); -} - void HangWatchState::SetIgnoreCurrentWatchHangsInScope() { deadline_.SetIgnoreCurrentWatchHangsInScope(); } @@ -1109,7 +1060,7 @@ HangWatchState::GetHangWatchStateForCurrentThread() { return hang_watch_state.get(); } -PlatformThreadId HangWatchState::GetThreadID() const { +uint64_t HangWatchState::GetThreadID() const { return thread_id_; } diff --git a/chromium/base/threading/hang_watcher.h b/chromium/base/threading/hang_watcher.h index 99536d742fc..9ba7f494f4e 100644 --- a/chromium/base/threading/hang_watcher.h +++ b/chromium/base/threading/hang_watcher.h @@ -18,7 +18,6 @@ #include "base/callback_helpers.h" #include "base/compiler_specific.h" #include "base/debug/crash_logging.h" -#include "base/feature_list.h" #include "base/memory/memory_pressure_listener.h" #include "base/synchronization/lock.h" #include "base/thread_annotations.h" @@ -86,11 +85,9 @@ class BASE_EXPORT WatchHangsInScope { // destroyed. TimeTicks previous_deadline_; - // Indicates whether the kIgnoreCurrentHang flag must be set upon exiting this - // WatchHangsInScope. This is true if the - // kIgnoreCurrentWatchHangsInScope flag was set upon entering this scope, - // but was cleared for this WatchHangsInScope because there was no active - // IgnoreHangsInScope. + // Indicates whether the kIgnoreCurrentWatchHangsInScope flag must be set upon + // exiting this WatchHangsInScope if a call to InvalidateActiveExpectations() + // previously suspended hang watching. bool set_hangs_ignored_on_exit_ = false; #if DCHECK_IS_ON() @@ -99,56 +96,12 @@ class BASE_EXPORT WatchHangsInScope { #endif }; -// Scoped object that disables hang watching on the thread. The object nullifies -// the effect of all live WatchHangsInScope instances and also that of new -// WatchHangsInScope instances created during its lifetime. Use to avoid -// capturing hangs for operations known to take unbounded time like waiting for -// user input. This does not unregister the thread so when this object is -// destroyed hang watching resumes for new WatchHangsInScopes. -// -// Example usage: -// { -// WatchHangsInScope scope_1; -// { -// WatchHangsInScope scope_2; -// IgnoreHangsInScope disabler; -// WaitForUserInput(); -// WatchHangsInScope scope_3; -// } -// -// WatchHangsInScope scope_4; -// } -// -// WatchHangsInScope scope_5; -// -// In this example hang watching is disabled for WatchHangsInScopes 1, 2 and -// 3 since they were either active at the time of the |disabler|'s creation or -// created while the disabler was still active. WatchHangsInScopes 4 and 5 -// are unaffected since they were created after the disabler was destroyed. -// -class BASE_EXPORT IgnoreHangsInScope { - public: - IgnoreHangsInScope(); - ~IgnoreHangsInScope(); - IgnoreHangsInScope(const IgnoreHangsInScope&) = delete; - IgnoreHangsInScope& operator=(const IgnoreHangsInScope&) = delete; - - private: - // Will be true if the object actually disabled hang watching and - // false if watching was already disabled by a previously declared object. - bool took_effect_ = true; -}; - // Monitors registered threads for hangs by inspecting their associated // HangWatchStates for deadline overruns. This happens at a regular interval on // a separate thread. Only one instance of HangWatcher can exist at a time // within a single process. This instance must outlive all monitored threads. class BASE_EXPORT HangWatcher : public DelegateSimpleThread::Delegate { public: - // Determines if the HangWatcher is activated. When false the HangWatcher - // thread never started. - static const base::Feature kEnableHangWatcher; - // Describes the type of a thread for logging purposes. enum class ThreadType { kIOThread = 0, @@ -185,11 +138,36 @@ class BASE_EXPORT HangWatcher : public DelegateSimpleThread::Delegate { static bool IsEnabled(); static bool IsThreadPoolHangWatchingEnabled(); static bool IsIOThreadHangWatchingEnabled(); - static bool IsUIThreadHangWatchingEnabled(); // Returns true if crash dump reporting is configured for any thread type. static bool IsCrashReportingEnabled(); + // Use to avoid capturing hangs for operations known to take unbounded time + // like waiting for user input. WatchHangsInScope objects created after this + // call will take effect. To resume watching for hangs create a new + // WatchHangsInScope after the unbounded operation finishes. + // + // Example usage: + // { + // WatchHangsInScope scope_1; + // { + // WatchHangsInScope scope_2; + // InvalidateActiveExpectations(); + // WaitForUserInput(); + // } + // + // WatchHangsInScope scope_4; + // } + // + // WatchHangsInScope scope_5; + // + // In this example hang watching is disabled for WatchHangsInScopes 1 and 2 + // since they were both active at the time of the invalidation. + // WatchHangsInScopes 4 and 5 are unaffected since they were created after the + // end of the WatchHangsInScope that was current at the time of invalidation. + // + static void InvalidateActiveExpectations(); + // Sets up the calling thread to be monitored for threads. Returns a // ScopedClosureRunner that unregisters the thread. This closure has to be // called from the registered thread before it's joined. Returns a null @@ -404,13 +382,10 @@ class BASE_EXPORT HangWatchDeadline { enum class Flag : uint64_t { // Minimum value for validation purposes. Not currently used. kMinValue = bits::LeftmostBit<uint64_t>() >> 7, - // Persistent because control by the lifetime of IgnoreHangsInScope. - kHasActiveIgnoreHangsInScope = bits::LeftmostBit<uint64_t>() >> 2, // Persistent because if hang detection is disabled on a thread it should // be re-enabled manually. kIgnoreCurrentWatchHangsInScope = bits::LeftmostBit<uint64_t>() >> 1, - // Non-persistent because a new value means a new WatchHangsInScope - // started + // Non-persistent because a new value means a new WatchHangsInScope started // after the beginning of capture. It can't be implicated in the hang so we // don't want it to block. kShouldBlockOnHang = bits::LeftmostBit<uint64_t>() >> 0, @@ -455,12 +430,6 @@ class BASE_EXPORT HangWatchDeadline { // not set the flag and returns false. bool SetShouldBlockOnHang(uint64_t old_flags, TimeTicks old_deadline); - // Sets the kHasActiveIgnoreHangsInScope flag. - void SetHasActiveIgnoreHangsInScope(); - - // Clears the kHasActiveIgnoreHangsInScope flag. - void UnsetHasActiveIgnoreHangsInScope(); - // Sets the kIgnoreCurrentWatchHangsInScope flag. void SetIgnoreCurrentWatchHangsInScope(); @@ -585,22 +554,14 @@ class BASE_EXPORT HangWatchState { // Sets the deadline to a new value. void SetDeadline(TimeTicks deadline); - // Mark this thread as ignored for hang watching. This means existing hang - // watch will not trigger hangs. + // Mark this thread as ignored for hang watching. This means existing + // WatchHangsInScope will not trigger hangs. void SetIgnoreCurrentWatchHangsInScope(); // Reactivate hang watching on this thread. Should be called when all // WatchHangsInScope instances that were ignored have completed. void UnsetIgnoreCurrentWatchHangsInScope(); - // Mark hang watching as disabled on this thread. This means new - // WatchHangsInScope instances will not trigger hangs. - void SetHasActiveIgnoreHangsInScope(); - - // Reactivate hang watching on this thread. New WatchHangsInScope - // instances will trigger hangs. - void UnsetHasActiveIgnoreHangsInScope(); - // Mark the current state as having to block in its destruction until hang // capture completes. bool SetShouldBlockOnHang(uint64_t old_flags, TimeTicks old_deadline); @@ -622,7 +583,7 @@ class BASE_EXPORT HangWatchState { WatchHangsInScope* GetCurrentWatchHangsInScope(); #endif - PlatformThreadId GetThreadID() const; + uint64_t GetThreadID() const; // Retrieve the current hang watch deadline directly. For testing only. HangWatchDeadline* GetHangWatchDeadlineForTesting(); @@ -648,7 +609,10 @@ class BASE_EXPORT HangWatchState { // reaches the value contained in it this constistutes a hang. HangWatchDeadline deadline_; - const PlatformThreadId thread_id_; + // A unique ID of the thread under watch. Used for logging in crash reports + // only. Unsigned type is used as it provides a correct behavior for all + // platforms for positive thread ids. Any valid thread id should be positive. + uint64_t thread_id_; // Number of active HangWatchScopeEnables on this thread. int nesting_level_ = 0; diff --git a/chromium/base/threading/hang_watcher_unittest.cc b/chromium/base/threading/hang_watcher_unittest.cc index 312c0a37c9c..b2aa3593ee4 100644 --- a/chromium/base/threading/hang_watcher_unittest.cc +++ b/chromium/base/threading/hang_watcher_unittest.cc @@ -22,6 +22,7 @@ #include "base/test/test_timeouts.h" #include "base/threading/platform_thread.h" #include "base/threading/thread_checker.h" +#include "base/threading/threading_features.h" #include "base/time/tick_clock.h" #include "base/time/time.h" #include "build/build_config.h" @@ -38,8 +39,8 @@ namespace { // Use with a FeatureList to activate crash dumping for threads marked as // threadpool threads. const std::vector<base::test::ScopedFeatureList::FeatureAndParams> - kFeatureAndParams{{base::HangWatcher::kEnableHangWatcher, - {{"ui_thread_log_level", "2"}}}}; + kFeatureAndParams{ + {base::kEnableHangWatcher, {{"ui_thread_log_level", "2"}}}}; // Use this value to mark things very far off in the future. Adding this // to TimeTicks::Now() gives a point that will never be reached during the @@ -144,12 +145,15 @@ class HangWatcherTest : public testing::Test { base::test::ScopedFeatureList feature_list_; - HangWatcher hang_watcher_; - // Used exclusively for MOCK_TIME. No tasks will be run on the environment. // Single threaded to avoid ThreadPool WorkerThreads registering. test::SingleThreadTaskEnvironment task_environment_{ test::TaskEnvironment::TimeSource::MOCK_TIME}; + + // This must be declared last (after task_environment_, for example) so that + // the watcher thread is joined before objects like the mock timer are + // destroyed, causing racy crashes. + HangWatcher hang_watcher_; }; class HangWatcherBlockingThreadTest : public HangWatcherTest { @@ -202,210 +206,129 @@ class HangWatcherBlockingThreadTest : public HangWatcherTest { }; } // namespace -// Regression test for crbug.com/1196285 -TEST_F(HangWatcherTest, MultipleIgnoreHangsInScopeDoNotCancelOut) { +TEST_F(HangWatcherTest, InvalidatingExpectationsPreventsCapture) { // Register the main test thread for hang watching. auto unregister_thread_closure = HangWatcher::RegisterThread(base::HangWatcher::ThreadType::kUIThread); - // De-activate hang watching, - IgnoreHangsInScope disabler; - - { - // Redundently de-activate hang watching. - IgnoreHangsInScope disabler_2; - } - // Create a hang. WatchHangsInScope expires_instantly(base::TimeDelta{}); task_environment_.FastForwardBy(kHangTime); + // de-activate hang watching, + base::HangWatcher::InvalidateActiveExpectations(); + // Trigger a monitoring on HangWatcher thread and verify results. - // Hang is not detected since it was covered by a disabler object. The - // creation and destruction of |disabler_2| should not affect that. + // Hang is not detected. hang_watcher_.SignalMonitorEventForTesting(); monitor_event_.Wait(); ASSERT_FALSE(hang_event_.IsSignaled()); } -TEST_F( - HangWatcherTest, - ScopeDisabledCreateScopeDisabledDestroyScopeEnabledCreateScopeEnabledDestroy) { +TEST_F(HangWatcherTest, MultipleInvalidateExpectationsDoNotCancelOut) { // Register the main test thread for hang watching. auto unregister_thread_closure = HangWatcher::RegisterThread(base::HangWatcher::ThreadType::kUIThread); - { - WatchHangsInScope expires_instantly(base::TimeDelta{}); - task_environment_.FastForwardBy(kHangTime); + // Create a hang. + WatchHangsInScope expires_instantly(base::TimeDelta{}); + task_environment_.FastForwardBy(kHangTime); - // De-activate hang watching, - IgnoreHangsInScope disabler; - } + // de-activate hang watching, + base::HangWatcher::InvalidateActiveExpectations(); - WatchHangsInScope also_expires_instantly(base::TimeDelta{}); - task_environment_.FastForwardBy(kHangTime); + // Redundently de-activate hang watching. + base::HangWatcher::InvalidateActiveExpectations(); // Trigger a monitoring on HangWatcher thread and verify results. + // Hang is not detected. hang_watcher_.SignalMonitorEventForTesting(); monitor_event_.Wait(); - - // Hang is detected since the new WatchHangsInScope was not covered by the - // disabler. - monitor_event_.Wait(); - ASSERT_TRUE(hang_event_.IsSignaled()); + ASSERT_FALSE(hang_event_.IsSignaled()); } -TEST_F( - HangWatcherTest, - ScopeEnabledCreateScopeDisabledCreateScopeEnabledDestroyScopeDisabledDestroy) { +TEST_F(HangWatcherTest, NewInnerWatchHangsInScopeAfterInvalidationDetectsHang) { // Register the main test thread for hang watching. auto unregister_thread_closure = HangWatcher::RegisterThread(base::HangWatcher::ThreadType::kUIThread); - absl::optional<IgnoreHangsInScope> disabler; + WatchHangsInScope expires_instantly(base::TimeDelta{}); + task_environment_.FastForwardBy(kHangTime); + + // De-activate hang watching. + base::HangWatcher::InvalidateActiveExpectations(); - // De-activate hang watching, { - // Start a WatchHangsInScope that expires right away. Then advance - // time to make sure a hang is detected. - WatchHangsInScope expires_instantly(base::TimeDelta{}); + WatchHangsInScope also_expires_instantly(base::TimeDelta{}); task_environment_.FastForwardBy(kHangTime); - disabler.emplace(); - // Trigger a monitoring on HangWatcher thread and verify results. hang_watcher_.SignalMonitorEventForTesting(); monitor_event_.Wait(); - } - - disabler.reset(); - - // Hang is ignored since a disabler was live during the lifetime of the hung - // WatchHangsInScope. - ASSERT_FALSE(hang_event_.IsSignaled()); -} - -TEST_F( - HangWatcherTest, - ScopeDisabledCreateScopeEnabledCreateScopeDisabledDestroyScopeEnabledDestroy) { - // Register the main test thread for hang watching. - auto unregister_thread_closure = - HangWatcher::RegisterThread(base::HangWatcher::ThreadType::kUIThread); - - absl::optional<IgnoreHangsInScope> disabler; - // De-activate hang watching, - { - disabler.emplace(); - - // Start a WatchHangsInScope that expires right away. Then advance - // time to make sure a hang is detected. - WatchHangsInScope expires_instantly(base::TimeDelta{}); - task_environment_.FastForwardBy(kHangTime); - - disabler.reset(); - - // Trigger a monitoring on HangWatcher thread and verify results. - hang_watcher_.SignalMonitorEventForTesting(); + // Hang is detected since the new WatchHangsInScope temporarily + // re-activated hang_watching. monitor_event_.Wait(); + ASSERT_TRUE(hang_event_.IsSignaled()); } - // Hang is ignored since a disabler was live during the lifetime of the hung - // WatchHangsInScope. - ASSERT_FALSE(hang_event_.IsSignaled()); -} - -TEST_F( - HangWatcherTest, - ScopeDisabledCreateScopeEnabledCreateScopeEnabledDestroyScopeDisabledDestroy) { - // Register the main test thread for hang watching. - auto unregister_thread_closure = - HangWatcher::RegisterThread(base::HangWatcher::ThreadType::kUIThread); - - // De-activate hang watching, - IgnoreHangsInScope disabler; - { - // Start a WatchHangsInScope that expires right away. Then advance - // time to make sure a hang is detected. - WatchHangsInScope expires_instantly(base::TimeDelta{}); - task_environment_.FastForwardBy(kHangTime); + // Reset to attempt capture again. + monitor_event_.Reset(); + hang_event_.Reset(); - // Trigger a monitoring on HangWatcher thread and verify results. - hang_watcher_.SignalMonitorEventForTesting(); - monitor_event_.Wait(); - } + // Trigger a monitoring on HangWatcher thread and verify results. + hang_watcher_.SignalMonitorEventForTesting(); + monitor_event_.Wait(); - // Hang is ignored. + // Hang is not detected since execution is back to being covered by + // |expires_instantly| for which expectations were invalidated. + monitor_event_.Wait(); ASSERT_FALSE(hang_event_.IsSignaled()); } -TEST_F(HangWatcherTest, ScopeCreateTempCreateTempDestroyScopeDestroy) { +TEST_F(HangWatcherTest, + NewSeparateWatchHangsInScopeAfterInvalidationDetectsHang) { // Register the main test thread for hang watching. auto unregister_thread_closure = HangWatcher::RegisterThread(base::HangWatcher::ThreadType::kUIThread); + { - // Start a WatchHangsInScope that expires right away. Then advance - // time to make sure a hang is detected. WatchHangsInScope expires_instantly(base::TimeDelta{}); task_environment_.FastForwardBy(kHangTime); - { - // De-activate hang watching, - IgnoreHangsInScope disabler; - } - - // Trigger a monitoring on HangWatcher thread and verify results. - hang_watcher_.SignalMonitorEventForTesting(); - monitor_event_.Wait(); + // De-activate hang watching. + base::HangWatcher::InvalidateActiveExpectations(); } - // Hang is ignored. - ASSERT_FALSE(hang_event_.IsSignaled()); -} - -TEST_F( - HangWatcherTest, - ScopeEnabledCreateScopeDisabledCreateScopeDisabledDestroyScopeEnabledDestroy) { - // Register the main test thread for hang watching. - auto unregister_thread_closure = - HangWatcher::RegisterThread(base::HangWatcher::ThreadType::kUIThread); - { - // Start a WatchHangsInScope that expires right away. Then advance - // time to make sure a hang is detected. - WatchHangsInScope expires_instantly(base::TimeDelta{}); - task_environment_.FastForwardBy(kHangTime); - - // De-activate hang watching, - IgnoreHangsInScope disabler; + WatchHangsInScope also_expires_instantly(base::TimeDelta{}); + task_environment_.FastForwardBy(kHangTime); - // Trigger a monitoring on HangWatcher thread and verify results. - hang_watcher_.SignalMonitorEventForTesting(); - monitor_event_.Wait(); - } + // Trigger a monitoring on HangWatcher thread and verify results. + hang_watcher_.SignalMonitorEventForTesting(); + monitor_event_.Wait(); - // Hang is ignored. - ASSERT_FALSE(hang_event_.IsSignaled()); + // Hang is detected since the new WatchHangsInScope did not have its + // expectations invalidated. + monitor_event_.Wait(); + ASSERT_TRUE(hang_event_.IsSignaled()); } -// Test that disabling an inner WatchHangsInScope will also prevent hang -// detection in outer scopes. +// Test that invalidating expectations from inner WatchHangsInScope will also +// prevent hang detection in outer scopes. TEST_F(HangWatcherTest, ScopeDisabledObjectInnerScope) { // Register the main test thread for hang watching. auto unregister_thread_closure = HangWatcher::RegisterThread(base::HangWatcher::ThreadType::kUIThread); // Start a WatchHangsInScope that expires right away. Then advance - // time to make sure a hang is detected. + // time to make sure no hang is detected. WatchHangsInScope expires_instantly(base::TimeDelta{}); task_environment_.FastForwardBy(kHangTime); { - // De-activate hang watching, - IgnoreHangsInScope disabler; - - // Start a WatchHangsInScope under the disabler that expires right away. - // Then advance time to make sure a hang is detected. WatchHangsInScope also_expires_instantly(base::TimeDelta{}); + + // De-activate hang watching. + base::HangWatcher::InvalidateActiveExpectations(); task_environment_.FastForwardBy(kHangTime); } @@ -424,19 +347,18 @@ TEST_F(HangWatcherTest, NewScopeAfterDisabling) { HangWatcher::RegisterThread(base::HangWatcher::ThreadType::kUIThread); // Start a WatchHangsInScope that expires right away. Then advance - // time to make sure a hang is detected. + // time to make sure no hang is detected. WatchHangsInScope expires_instantly(base::TimeDelta{}); task_environment_.FastForwardBy(kHangTime); { - // De-activate hang watching, - IgnoreHangsInScope disabler; - - // Start a WatchHangsInScope under the disabler that expires right away. - // Then advance time to make sure a hang is detected. WatchHangsInScope also_expires_instantly(base::TimeDelta{}); + + // De-activate hang watching. + base::HangWatcher::InvalidateActiveExpectations(); task_environment_.FastForwardBy(kHangTime); } + // New scope for which expecations are never invalidated. WatchHangsInScope also_expires_instantly(base::TimeDelta{}); task_environment_.FastForwardBy(kHangTime); @@ -1078,10 +1000,9 @@ TEST_F(WatchHangsInScopeBlockingTest, MAYBE_NewScopeDoesNotBlockDuringCapture) { namespace internal { namespace { -constexpr std::array<HangWatchDeadline::Flag, 4> kAllFlags{ +constexpr std::array<HangWatchDeadline::Flag, 3> kAllFlags{ {HangWatchDeadline::Flag::kMinValue, HangWatchDeadline::Flag::kIgnoreCurrentWatchHangsInScope, - HangWatchDeadline::Flag::kHasActiveIgnoreHangsInScope, HangWatchDeadline::Flag::kShouldBlockOnHang}}; } // namespace diff --git a/chromium/base/threading/platform_thread.cc b/chromium/base/threading/platform_thread.cc index 0fc082fabf2..23eb14f1d21 100644 --- a/chromium/base/threading/platform_thread.cc +++ b/chromium/base/threading/platform_thread.cc @@ -6,6 +6,7 @@ #include <atomic> #include <memory> +#include <ostream> #include "base/feature_list.h" #include "base/time/time.h" @@ -28,6 +29,11 @@ std::atomic<bool> g_use_thread_priorities(true); } // namespace +std::ostream& operator<<(std::ostream& os, const PlatformThreadRef& ref) { + os << ref.id_; + return os; +} + // static void PlatformThread::SetCurrentThreadPriority(ThreadPriority priority) { if (g_use_thread_priorities.load()) diff --git a/chromium/base/threading/platform_thread.h b/chromium/base/threading/platform_thread.h index eb0428f64d4..eefad38e2bc 100644 --- a/chromium/base/threading/platform_thread.h +++ b/chromium/base/threading/platform_thread.h @@ -11,6 +11,8 @@ #include <stddef.h> +#include <iosfwd> + #include "base/base_export.h" #include "base/macros.h" #include "base/time/time.h" @@ -56,7 +58,7 @@ class PlatformThreadRef { #else // OS_POSIX typedef pthread_t RefType; #endif - constexpr PlatformThreadRef() : id_(0) {} + constexpr PlatformThreadRef() = default; explicit constexpr PlatformThreadRef(RefType id) : id_(id) {} @@ -69,10 +71,17 @@ class PlatformThreadRef { bool is_null() const { return id_ == 0; } + private: - RefType id_; + friend BASE_EXPORT std::ostream& operator<<(std::ostream& os, + const PlatformThreadRef& ref); + + RefType id_ = 0; }; +BASE_EXPORT std::ostream& operator<<(std::ostream& os, + const PlatformThreadRef& ref); + // Used to operate on threads. class PlatformThreadHandle { public: @@ -216,7 +225,7 @@ class BASE_EXPORT PlatformThread { // A thread may not be able to raise its priority back up after lowering it if // the process does not have a proper permission, e.g. CAP_SYS_NICE on Linux. // A thread may not be able to lower its priority back down after raising it - // to REALTIME_AUDIO. + // to DISPLAY or REALTIME_AUDIO. // // This function must not be called from the main thread on Mac. This is to // avoid performance regressions (https://crbug.com/601270). diff --git a/chromium/base/threading/platform_thread_fuchsia.cc b/chromium/base/threading/platform_thread_fuchsia.cc index 97de890b8a3..a558c94bdc1 100644 --- a/chromium/base/threading/platform_thread_fuchsia.cc +++ b/chromium/base/threading/platform_thread_fuchsia.cc @@ -8,11 +8,82 @@ #include <sched.h> #include <zircon/syscalls.h> +#include <fuchsia/media/cpp/fidl.h> +#include <lib/fdio/directory.h> +#include <lib/sys/cpp/component_context.h> + +#include "base/fuchsia/fuchsia_logging.h" +#include "base/fuchsia/process_context.h" +#include "base/fuchsia/scheduler.h" +#include "base/no_destructor.h" #include "base/threading/platform_thread_internal_posix.h" #include "base/threading/thread_id_name_manager.h" +#include "base/threading/thread_local_storage.h" namespace base { +namespace { + +fuchsia::media::ProfileProviderSyncPtr ConnectProfileProvider() { + fuchsia::media::ProfileProviderSyncPtr profile_provider; + base::ComponentContextForProcess()->svc()->Connect( + profile_provider.NewRequest()); + return profile_provider; +} + +void ScheduleAsMediaThread(StringPiece name, TimeDelta period, float capacity) { + DCHECK(!period.is_zero()); + DCHECK_GT(capacity, 0.0); + DCHECK_LT(capacity, 1.0); + + static const base::NoDestructor<fuchsia::media::ProfileProviderSyncPtr> + profile_provider(ConnectProfileProvider()); + + zx::thread dup_thread; + zx_status_t status = + zx::thread::self()->duplicate(ZX_RIGHT_SAME_RIGHTS, &dup_thread); + ZX_CHECK(status == ZX_OK, status) << "zx_object_duplicate"; + + int64_t out_period, out_capacity; + status = (*profile_provider) + ->RegisterHandlerWithCapacity( + std::move(dup_thread), std::string(name), + period.ToZxDuration(), capacity, &out_period, &out_capacity); + + if (status != ZX_OK) { + ZX_LOG(WARNING, status) + << "Failed to register a realtime thread. Is " + "fuchsia.media.ProfileProvider in the component sandbox?"; + } +} + +// Return ThreadLocalStorage slot used to store priority of the current thread. +// The value is stored as an integer value converted to a pointer. 1 is added to +// the integer value in order to distinguish the case when the TLS slot is not +// initialized. +base::ThreadLocalStorage::Slot* GetThreadPriorityTlsSlot() { + static base::NoDestructor<base::ThreadLocalStorage::Slot> tls_slot; + return tls_slot.get(); +} + +void SaveThreadPriorityToTls(ThreadPriority priority) { + GetThreadPriorityTlsSlot()->Set( + reinterpret_cast<void*>(static_cast<uintptr_t>(priority) + 1)); +} + +ThreadPriority GetThreadPriorityFromTls() { + uintptr_t value = + reinterpret_cast<uintptr_t>(GetThreadPriorityTlsSlot()->Get()); + + // Thread priority is set to NORMAL by default. + if (value == 0) + return ThreadPriority::NORMAL; + + return static_cast<ThreadPriority>(value - 1); +} + +} // namespace + void InitThreading() {} void TerminateOnThread() {} @@ -32,17 +103,35 @@ void PlatformThread::SetName(const std::string& name) { // static bool PlatformThread::CanIncreaseThreadPriority(ThreadPriority priority) { - return false; + return true; } // static void PlatformThread::SetCurrentThreadPriorityImpl(ThreadPriority priority) { - // TODO(https://crbug.com/926583): Fuchsia does not currently support this. + switch (priority) { + case ThreadPriority::BACKGROUND: + case ThreadPriority::NORMAL: + DCHECK(GetThreadPriorityFromTls() != ThreadPriority::DISPLAY && + GetThreadPriorityFromTls() != ThreadPriority::REALTIME_AUDIO); + break; + + case ThreadPriority::DISPLAY: + ScheduleAsMediaThread("chromium.base.threading.display", + kDisplaySchedulingPeriod, kAudioSchedulingCapacity); + break; + + case ThreadPriority::REALTIME_AUDIO: + ScheduleAsMediaThread("chromium.base.threading.realtime-audio", + kAudioSchedulingPeriod, kDisplaySchedulingCapacity); + break; + } + + SaveThreadPriorityToTls(priority); } // static ThreadPriority PlatformThread::GetCurrentThreadPriority() { - return ThreadPriority::NORMAL; + return GetThreadPriorityFromTls(); } } // namespace base diff --git a/chromium/base/threading/platform_thread_posix.cc b/chromium/base/threading/platform_thread_posix.cc index dbf96f68f9d..e6c8ec66ab8 100644 --- a/chromium/base/threading/platform_thread_posix.cc +++ b/chromium/base/threading/platform_thread_posix.cc @@ -19,7 +19,6 @@ #include "base/debug/activity_tracker.h" #include "base/lazy_instance.h" #include "base/logging.h" -#include "base/no_destructor.h" #include "base/threading/platform_thread_internal_posix.h" #include "base/threading/scoped_blocking_call.h" #include "base/threading/thread_id_name_manager.h" @@ -195,7 +194,7 @@ PlatformThreadId PlatformThread::CurrentId() { #if defined(OS_APPLE) return pthread_mach_thread_np(pthread_self()); #elif defined(OS_LINUX) || defined(OS_CHROMEOS) - static NoDestructor<InitAtFork> init_at_fork; + static InitAtFork init_at_fork; if (g_thread_id == -1) { g_thread_id = syscall(__NR_gettid); } else { @@ -206,6 +205,11 @@ PlatformThreadId PlatformThread::CurrentId() { } return g_thread_id; #elif defined(OS_ANDROID) + // Note: do not cache the return value inside a thread_local variable on + // Android (as above). The reasons are: + // - thread_local is slow on Android (goes through emutls) + // - gettid() is fast, since its return value is cached in pthread (in the + // thread control block of pthread). See gettid.c in bionic. return gettid(); #elif defined(OS_FUCHSIA) return zx_thread_self(); diff --git a/chromium/base/threading/platform_thread_unittest.cc b/chromium/base/threading/platform_thread_unittest.cc index 00aab828211..43a80220fdb 100644 --- a/chromium/base/threading/platform_thread_unittest.cc +++ b/chromium/base/threading/platform_thread_unittest.cc @@ -5,8 +5,8 @@ #include <stddef.h> #include "base/compiler_specific.h" +#include "base/cxx17_backports.h" #include "base/process/process.h" -#include "base/stl_util.h" #include "base/synchronization/waitable_event.h" #include "base/test/scoped_feature_list.h" #include "base/threading/platform_thread.h" @@ -317,9 +317,6 @@ TEST(PlatformThreadTest, CanIncreaseThreadPriority) { // On Ubuntu, RLIMIT_NICE and RLIMIT_RTPRIO are 0 by default, so we won't be // able to increase priority to any level. constexpr bool kCanIncreasePriority = false; -#elif defined(OS_FUCHSIA) - // Fuchsia doesn't support thread priorities. - constexpr bool kCanIncreasePriority = false; #else constexpr bool kCanIncreasePriority = true; #endif diff --git a/chromium/base/threading/scoped_blocking_call_internal.cc b/chromium/base/threading/scoped_blocking_call_internal.cc index 1516230f048..b1f8de61fd2 100644 --- a/chromium/base/threading/scoped_blocking_call_internal.cc +++ b/chromium/base/threading/scoped_blocking_call_internal.cc @@ -56,7 +56,20 @@ void ClearBlockingObserverForCurrentThread() { IOJankMonitoringWindow::ScopedMonitoredCall::ScopedMonitoredCall() : call_start_(TimeTicks::Now()), - assigned_jank_window_(MonitorNextJankWindowIfNecessary(call_start_)) {} + assigned_jank_window_(MonitorNextJankWindowIfNecessary(call_start_)) { + if (assigned_jank_window_) { + // TimeTicks using a monotonic clock and MonitorNextJankWindowIfNecessary + // synchronizing via a lock to return |assigned_jank_window_| was initially + // believed to guarantee that |call_start_| is either equal or beyond + // |assigned_jank_window_->start_time_|. Violating this assumption can + // result in negative indexing and OOB-writes in AddJank(). + // We now know this assumption can be violated. This condition hotfixes + // the issue by discarding ScopedMonitoredCalls where it occurs. + // TODO(crbug.com/1209622): Implement a proper fix. + if (call_start_ < assigned_jank_window_->start_time_) + assigned_jank_window_.reset(); + } +} IOJankMonitoringWindow::ScopedMonitoredCall::~ScopedMonitoredCall() { if (assigned_jank_window_) { @@ -187,6 +200,11 @@ IOJankMonitoringWindow::~IOJankMonitoringWindow() NO_THREAD_SAFETY_ANALYSIS { void IOJankMonitoringWindow::OnBlockingCallCompleted(TimeTicks call_start, TimeTicks call_end) { + // Confirm we never hit a case of TimeTicks going backwards on the same thread + // nor of TimeTicks rolling over the int64_t boundary (which would break + // comparison operators). + DCHECK_LE(call_start, call_end); + if (call_end - call_start < kIOJankInterval) return; @@ -210,6 +228,9 @@ void IOJankMonitoringWindow::OnBlockingCallCompleted(TimeTicks call_start, void IOJankMonitoringWindow::AddJank(int local_jank_start_index, int num_janky_intervals) { + DCHECK_GE(local_jank_start_index, 0); + DCHECK_LT(local_jank_start_index, kNumIntervals); + // Increment jank counts for intervals in this window. If // |num_janky_intervals| lands beyond kNumIntervals, the additional intervals // will be reported to |next_|. diff --git a/chromium/base/threading/scoped_blocking_call_internal.h b/chromium/base/threading/scoped_blocking_call_internal.h index 60726261f16..e149bc8edb4 100644 --- a/chromium/base/threading/scoped_blocking_call_internal.h +++ b/chromium/base/threading/scoped_blocking_call_internal.h @@ -93,6 +93,12 @@ class BASE_EXPORT IOJankMonitoringWindow static constexpr TimeDelta kTimeDiscrepancyTimeout = kIOJankInterval * 10; static constexpr int kNumIntervals = kMonitoringWindow / kIOJankInterval; + // kIOJankIntervals must integrally fill kMonitoringWindow + static_assert((kMonitoringWindow % kIOJankInterval).is_zero(), ""); + + // Cancelation is simple because it can only affect the current window. + static_assert(kTimeDiscrepancyTimeout < kMonitoringWindow, ""); + private: friend class base::RefCountedThreadSafe<IOJankMonitoringWindow>; friend void base::EnableIOJankMonitoringForProcess(IOJankReportingCallback); diff --git a/chromium/base/threading/scoped_blocking_call_unittest.cc b/chromium/base/threading/scoped_blocking_call_unittest.cc index 942374aebab..aeb04392e50 100644 --- a/chromium/base/threading/scoped_blocking_call_unittest.cc +++ b/chromium/base/threading/scoped_blocking_call_unittest.cc @@ -21,6 +21,7 @@ #include "base/test/test_waitable_event.h" #include "base/threading/scoped_blocking_call_internal.h" #include "base/threading/thread_restrictions.h" +#include "base/time/time_override.h" #include "build/build_config.h" #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" @@ -164,7 +165,10 @@ TEST(ScopedBlockingCallDestructionOrderTest, InvalidDestructionOrder) { class ScopedBlockingCallIOJankMonitoringTest : public testing::Test { public: - ScopedBlockingCallIOJankMonitoringTest() = default; + explicit ScopedBlockingCallIOJankMonitoringTest( + test::TaskEnvironment::TimeSource time_source = + test::TaskEnvironment::TimeSource::MOCK_TIME) + : task_environment_(time_source) {} void SetUp() override { // Note 1: While EnableIOJankMonitoringForProcess() is documented as being @@ -208,10 +212,34 @@ class ScopedBlockingCallIOJankMonitoringTest : public testing::Test { // TaskEnvironment+MOCK_TIME advances the test in lock steps. std::vector<std::pair<int, int>> reports_; - test::TaskEnvironment task_environment_{ - test::TaskEnvironment::TimeSource::MOCK_TIME}; + test::TaskEnvironment task_environment_; }; +// Manually mocks time to be able to move it backwards. Uses inheritance to +// ensure the clock override outlives the entire lifetime of the main test +// fixture (or weird things happen on destruction). +class ScopedBlockingCallIOJankMonitoringManualMockTimeTest + : public subtle::ScopedTimeClockOverrides, + public ScopedBlockingCallIOJankMonitoringTest { + public: + // Initialized to Now() when the test starts and manually controlled from + // there. + static TimeTicks reported_ticks_; + + ScopedBlockingCallIOJankMonitoringManualMockTimeTest() + : subtle::ScopedTimeClockOverrides( + nullptr, + []() { return reported_ticks_; }, + nullptr), + ScopedBlockingCallIOJankMonitoringTest( + test::TaskEnvironment::TimeSource::SYSTEM_TIME) { + reported_ticks_ = TimeTicks::Now(); + } +}; + +// static +TimeTicks ScopedBlockingCallIOJankMonitoringManualMockTimeTest::reported_ticks_; + TEST_F(ScopedBlockingCallIOJankMonitoringTest, Basic) { constexpr auto kJankTiming = internal::IOJankMonitoringWindow::kIOJankInterval * 7; @@ -847,4 +875,30 @@ TEST_F(ScopedBlockingCallIOJankMonitoringTest, EXPECT_THAT(reports_, ElementsAre(std::make_pair(0, 0))); } +// Regression test for crbug.com/1209622 +// Disabled due to flakiness on Linux and Fuchsia bots. +// https://crbug.com/1235945 +#if defined(OS_LINUX) || defined(OS_FUCHSIA) +#define MAYBE_IgnoreIfTickClockMovesBackwards \ + DISABLED_IgnoreIfTickClockMovesBackwards +#else +#define MAYBE_IgnoreIfTickClockMovesBackwards IgnoreIfTickClockMovesBackwards +#endif +TEST_F(ScopedBlockingCallIOJankMonitoringManualMockTimeTest, + MAYBE_IgnoreIfTickClockMovesBackwards) { + // Stomping 4 intervals in the past and 3 janky intervals from there is known + // to cause havoc (negatively indexing into |intervals_lock_|), per the + // original repro. Going back only 1 also causes negative indexing but not + // enough havoc to make this test crash without the fix. + { + reported_ticks_ -= internal::IOJankMonitoringWindow::kIOJankInterval * 4; + ScopedBlockingCall jank_in_past(FROM_HERE, BlockingType::MAY_BLOCK); + reported_ticks_ += internal::IOJankMonitoringWindow::kIOJankInterval * 3; + } + + // Force a report immediately, it should have ignored the jank in the past. + internal::IOJankMonitoringWindow::CancelMonitoringForTesting(); + EXPECT_THAT(reports_, ElementsAre(std::make_pair(0, 0))); +} + } // namespace base diff --git a/chromium/base/threading/sequence_local_storage_map.h b/chromium/base/threading/sequence_local_storage_map.h index 3857cc282d1..1ea010933f5 100644 --- a/chromium/base/threading/sequence_local_storage_map.h +++ b/chromium/base/threading/sequence_local_storage_map.h @@ -15,7 +15,7 @@ namespace internal { // A SequenceLocalStorageMap holds (slot_id) -> (value, destructor) items for a // sequence. When a task runs, it is expected that a pointer to its sequence's // SequenceLocalStorageMap is set in TLS using -// ScopedSetSequenceMapLocalStorageForCurrentThread. When a +// ScopedSetSequenceLocalStorageMapForCurrentThread. When a // SequenceLocalStorageMap is destroyed, it invokes the destructors associated // with values stored within it. // The Get() and Set() methods should not be accessed directly. diff --git a/chromium/base/threading/sequence_local_storage_slot_unittest.cc b/chromium/base/threading/sequence_local_storage_slot_unittest.cc index 7f146d49202..ad74f2611e0 100644 --- a/chromium/base/threading/sequence_local_storage_slot_unittest.cc +++ b/chromium/base/threading/sequence_local_storage_slot_unittest.cc @@ -6,8 +6,8 @@ #include <utility> +#include "base/cxx17_backports.h" #include "base/memory/ptr_util.h" -#include "base/stl_util.h" #include "base/threading/sequence_local_storage_map.h" #include "testing/gtest/include/gtest/gtest.h" diff --git a/chromium/base/threading/thread.cc b/chromium/base/threading/thread.cc index 8834896a59f..0454b1b0676 100644 --- a/chromium/base/threading/thread.cc +++ b/chromium/base/threading/thread.cc @@ -122,6 +122,22 @@ Thread::Options::Options(Options&& other) other.moved_from = true; } +Thread::Options& Thread::Options::operator=(Thread::Options&& other) { + DCHECK_NE(this, &other); + + message_pump_type = std::move(other.message_pump_type); + delegate = std::move(other.delegate); + timer_slack = std::move(other.timer_slack); + task_queue_time_domain = std::move(other.task_queue_time_domain); + message_pump_factory = std::move(other.message_pump_factory); + stack_size = std::move(other.stack_size); + priority = std::move(other.priority); + joinable = std::move(other.joinable); + other.moved_from = true; + + return *this; +} + Thread::Options::~Options() = default; Thread::Thread(const std::string& name) @@ -149,10 +165,10 @@ bool Thread::Start() { if (com_status_ == STA) options.message_pump_type = MessagePumpType::UI; #endif - return StartWithOptions(options); + return StartWithOptions(std::move(options)); } -bool Thread::StartWithOptions(const Options& options) { +bool Thread::StartWithOptions(Options options) { DCHECK(options.IsValid()); DCHECK(owning_sequence_checker_.CalledOnValidSequence()); DCHECK(!delegate_); @@ -175,7 +191,7 @@ bool Thread::StartWithOptions(const Options& options) { if (options.delegate) { DCHECK(!options.message_pump_factory); DCHECK(!options.task_queue_time_domain); - delegate_ = WrapUnique(options.delegate); + delegate_ = std::move(options.delegate); } else if (options.message_pump_factory) { delegate_ = std::make_unique<SequenceManagerThreadDelegate>( MessagePumpType::CUSTOM, options.message_pump_factory, diff --git a/chromium/base/threading/thread.h b/chromium/base/threading/thread.h index 72af7e36eb9..27ef5ffb0b5 100644 --- a/chromium/base/threading/thread.h +++ b/chromium/base/threading/thread.h @@ -80,7 +80,7 @@ class BASE_EXPORT Thread : PlatformThread::Delegate { Options(); Options(MessagePumpType type, size_t size); Options(Options&& other); - Options& operator=(const Options&& other) = delete; + Options& operator=(Options&& other); ~Options(); // Specifies the type of message pump that will be allocated on the thread. @@ -89,8 +89,7 @@ class BASE_EXPORT Thread : PlatformThread::Delegate { // An unbound Delegate that will be bound to the thread. Ownership // of |delegate| will be transferred to the thread. - // TODO(alexclarke): This should be a std::unique_ptr - Delegate* delegate = nullptr; + std::unique_ptr<Delegate> delegate = nullptr; // Specifies timer slack for thread message loop. TimerSlack timer_slack = TIMER_SLACK_NONE; @@ -171,7 +170,7 @@ class BASE_EXPORT Thread : PlatformThread::Delegate { // Note: This function can't be called on Windows with the loader lock held; // i.e. during a DllMain, global object construction or destruction, atexit() // callback. - bool StartWithOptions(const Options& options); + bool StartWithOptions(Options options); // Starts the thread and wait for the thread to start and run initialization // before returning. It's same as calling Start() and then diff --git a/chromium/base/threading/thread_id_name_manager.cc b/chromium/base/threading/thread_id_name_manager.cc index 73f1c96ddba..3d58b63b627 100644 --- a/chromium/base/threading/thread_id_name_manager.cc +++ b/chromium/base/threading/thread_id_name_manager.cc @@ -9,9 +9,9 @@ #include "base/check.h" #include "base/containers/contains.h" +#include "base/containers/cxx20_erase.h" #include "base/memory/singleton.h" #include "base/no_destructor.h" -#include "base/stl_util.h" #include "base/strings/string_util.h" #include "base/threading/thread_local.h" #include "base/trace_event/heap_profiler_allocation_context_tracker.h" // no-presubmit-check diff --git a/chromium/base/threading/thread_local_storage.h b/chromium/base/threading/thread_local_storage.h index 83e3be2c5db..59d5bb1d43c 100644 --- a/chromium/base/threading/thread_local_storage.h +++ b/chromium/base/threading/thread_local_storage.h @@ -17,10 +17,6 @@ #include <pthread.h> #endif -namespace ui { -class TLSDestructionCheckerForX11; -} - namespace base { class SamplingHeapProfiler; @@ -162,7 +158,6 @@ class BASE_EXPORT ThreadLocalStorage { friend class internal::ThreadLocalStorageTestInternal; friend class trace_event::MallocDumpProvider; friend class debug::GlobalActivityTracker; - friend class ui::TLSDestructionCheckerForX11; static bool HasBeenDestroyed(); DISALLOW_COPY_AND_ASSIGN(ThreadLocalStorage); diff --git a/chromium/base/threading/thread_restrictions.cc b/chromium/base/threading/thread_restrictions.cc index 071d8d0ff53..b4cf3c9ba78 100644 --- a/chromium/base/threading/thread_restrictions.cc +++ b/chromium/base/threading/thread_restrictions.cc @@ -294,6 +294,11 @@ ScopedAllowBaseSyncPrimitivesOutsideBlockingScope:: base::trace_event::InternedSourceLocation::Get( &ctx, base::trace_event::TraceSourceLocation(from_here))); }); + + // Since this object is used to indicate that sync primitives will be used to + // wait for an event ignore the current operation for hang watching purposes + // since the wait time duration is unknown. + base::HangWatcher::InvalidateActiveExpectations(); } ScopedAllowBaseSyncPrimitivesOutsideBlockingScope:: diff --git a/chromium/base/threading/thread_restrictions.h b/chromium/base/threading/thread_restrictions.h index 06faca58331..088e966c994 100644 --- a/chromium/base/threading/thread_restrictions.h +++ b/chromium/base/threading/thread_restrictions.h @@ -136,11 +136,14 @@ class WorkerThread; namespace scheduler { class WorkerThread; } -} +} // namespace blink namespace cc { class CompletionEvent; class TileTaskManagerImpl; } +namespace chromecast { +class CrashUtil; +} namespace chromeos { class BlockingMethodCaller; namespace system { @@ -173,6 +176,7 @@ class RenderWidgetHostViewMac; class SandboxHostLinux; class ScopedAllowWaitForDebugURL; class ServiceWorkerContextClient; +class ShellPathProvider; class SoftwareOutputDeviceMus; class SynchronousCompositor; class SynchronousCompositorHost; @@ -185,6 +189,9 @@ namespace cronet { class CronetPrefsManager; class CronetURLRequestContext; } // namespace cronet +namespace crosapi { +class LacrosThreadPriorityDelegate; +} // namespace crosapi namespace dbus { class Bus; } @@ -201,8 +208,8 @@ class ExecScriptScopedAllowBaseSyncPrimitives; namespace history_report { class HistoryReportJniBridge; } -namespace gpu { -class GpuChannelHost; +namespace ios_web_view { +class WebViewBrowserState; } namespace leveldb_env { class DBTracker; @@ -245,6 +252,7 @@ class LocalPrinterHandlerDefault; #if defined(OS_MAC) class PrintBackendServiceImpl; #endif +class PrintBackendServiceManager; class PrintJobWorker; class PrinterQuery; } @@ -281,6 +289,10 @@ namespace proxy_resolver { class ScopedAllowThreadJoinForProxyResolverV8Tracing; } +namespace remote_cocoa { +class DroppedScreenShotCopierMac; +} // namespace remote_cocoa + namespace remoting { class AutoThread; class ScopedBypassIOThreadRestrictions; @@ -405,14 +417,18 @@ class BASE_EXPORT ScopedAllowBlocking { friend class ash::MojoUtils; // http://crbug.com/1055467 friend class ash::BrowserDataMigrator; friend class blink::DiskDataAllocator; + friend class chromecast::CrashUtil; friend class content::BrowserProcessIOThread; friend class content::NetworkServiceInstancePrivate; friend class content::PepperPrintSettingsManagerImpl; friend class content::RenderProcessHostImpl; friend class content::RenderWidgetHostViewMac; // http://crbug.com/121917 + friend class content::ShellPathProvider; friend class content::WebContentsViewMac; friend class cronet::CronetPrefsManager; friend class cronet::CronetURLRequestContext; + friend class crosapi::LacrosThreadPriorityDelegate; + friend class ios_web_view::WebViewBrowserState; friend class memory_instrumentation::OSMetrics; friend class metrics::AndroidMetricsServiceClient; friend class module_installer::ScopedAllowModulePakLoad; @@ -421,7 +437,10 @@ class BASE_EXPORT ScopedAllowBlocking { #if defined(OS_MAC) friend class printing::PrintBackendServiceImpl; #endif + friend class printing::PrintBackendServiceManager; friend class printing::PrintJobWorker; + friend class remote_cocoa:: + DroppedScreenShotCopierMac; // https://crbug.com/1148078 friend class remoting::ScopedBypassIOThreadRestrictions; // crbug.com/1144161 friend class web::WebSubThread; friend class weblayer::BrowserContextImpl; @@ -575,7 +594,6 @@ class BASE_EXPORT ScopedAllowBaseSyncPrimitivesOutsideBlockingScope { friend class dbus::Bus; // http://crbug.com/125222 friend class disk_cache::BackendImpl; // http://crbug.com/74623 friend class disk_cache::InFlightIO; // http://crbug.com/74623 - friend class gpu::GpuChannelHost; // http://crbug.com/125264 friend class remoting::protocol:: ScopedAllowThreadJoinForWebRtcTransport; // http://crbug.com/660081 friend class midi::TaskService; // https://crbug.com/796830 @@ -601,11 +619,6 @@ class BASE_EXPORT ScopedAllowBaseSyncPrimitivesOutsideBlockingScope { std::unique_ptr<BooleanWithStack> was_disallowed_; #endif - // Since this object is used to indicate that sync primitives will be used to - // wait for an event ignore the current operation for hang watching purposes - // since the wait time duration is unknown. - base::IgnoreHangsInScope hang_watch_scope_disabled_; - DISALLOW_COPY_AND_ASSIGN(ScopedAllowBaseSyncPrimitivesOutsideBlockingScope); }; diff --git a/chromium/base/threading/thread_task_runner_handle.h b/chromium/base/threading/thread_task_runner_handle.h index 2a1ea638092..3fcd5935b49 100644 --- a/chromium/base/threading/thread_task_runner_handle.h +++ b/chromium/base/threading/thread_task_runner_handle.h @@ -10,7 +10,6 @@ #include "base/dcheck_is_on.h" #include "base/macros.h" #include "base/memory/scoped_refptr.h" -#include "base/optional.h" #include "base/run_loop.h" #include "base/single_thread_task_runner.h" #include "base/threading/sequenced_task_runner_handle.h" diff --git a/chromium/base/threading/thread_unittest.cc b/chromium/base/threading/thread_unittest.cc index 1a71f856dcb..229bcf3c69c 100644 --- a/chromium/base/threading/thread_unittest.cc +++ b/chromium/base/threading/thread_unittest.cc @@ -157,7 +157,7 @@ TEST_F(ThreadTest, StartWithOptions_StackSize) { #else options.stack_size = 3072 * sizeof(uintptr_t); #endif - EXPECT_TRUE(a.StartWithOptions(options)); + EXPECT_TRUE(a.StartWithOptions(std::move(options))); EXPECT_TRUE(a.task_runner()); EXPECT_TRUE(a.IsRunning()); @@ -180,7 +180,7 @@ TEST_F(ThreadTest, StartWithOptions_NonJoinable) { Thread::Options options; options.joinable = false; - EXPECT_TRUE(a->StartWithOptions(options)); + EXPECT_TRUE(a->StartWithOptions(std::move(options))); EXPECT_TRUE(a->task_runner()); EXPECT_TRUE(a->IsRunning()); @@ -246,7 +246,7 @@ TEST_F(ThreadTest, DISABLED_DestroyWhileRunningNonJoinableIsSafe) { Thread a("DestroyWhileRunningNonJoinableIsSafe"); Thread::Options options; options.joinable = false; - EXPECT_TRUE(a.StartWithOptions(options)); + EXPECT_TRUE(a.StartWithOptions(std::move(options))); EXPECT_TRUE(a.WaitUntilThreadStarted()); } @@ -362,7 +362,7 @@ TEST_F(ThreadTest, StartTwiceNonJoinableNotAllowed) { Thread::Options options; options.joinable = false; - EXPECT_TRUE(a->StartWithOptions(options)); + EXPECT_TRUE(a->StartWithOptions(std::move(options))); EXPECT_TRUE(a->task_runner()); EXPECT_TRUE(a->IsRunning()); @@ -585,14 +585,15 @@ class SequenceManagerThreadDelegate : public Thread::Delegate { TEST_F(ThreadTest, ProvidedThreadDelegate) { Thread thread("ThreadDelegate"); base::Thread::Options options; - options.delegate = new SequenceManagerThreadDelegate(); - thread.StartWithOptions(options); + options.delegate = std::make_unique<SequenceManagerThreadDelegate>(); - base::WaitableEvent event; + scoped_refptr<base::SingleThreadTaskRunner> task_runner = + options.delegate->GetDefaultTaskRunner(); + thread.StartWithOptions(std::move(options)); - options.delegate->GetDefaultTaskRunner()->PostTask( - FROM_HERE, - base::BindOnce(&base::WaitableEvent::Signal, base::Unretained(&event))); + base::WaitableEvent event; + task_runner->PostTask(FROM_HERE, base::BindOnce(&base::WaitableEvent::Signal, + base::Unretained(&event))); event.Wait(); thread.Stop(); diff --git a/chromium/base/threading/threading_features.h b/chromium/base/threading/threading_features.h index c8f6f0dc596..941818e819e 100644 --- a/chromium/base/threading/threading_features.h +++ b/chromium/base/threading/threading_features.h @@ -26,6 +26,8 @@ extern const BASE_EXPORT FeatureParam<double> kOptimizedRealtimeThreadingMacBusyLimit; #endif +extern const BASE_EXPORT Feature kEnableHangWatcher; + } // namespace base #endif // BASE_THREADING_THREADING_FEATURES_H_ |