Commit 773e8566 authored by Gabriel Charette's avatar Gabriel Charette Committed by Commit Bot

Introduce NoDestructorTest.PriorityInversionAtStaticInitializationResolves

I had voiced concerns about this when NoDestructor was introduced as a
strictly preferred alternative to LazyInstance @
https://groups.google.com/a/chromium.org/d/msg/cxx/rs_iizoAu6w/Gi6s0pmwBQAJ.
This CL migrates LazyInstanceTest.PriorityInversionAtInitializationResolves
to the NoDestructor thread-safe scoped-static-initialization paradigm.

It seems to be okay after all and this CL locks this in on all platforms.

Bug: 925323
Change-Id: I679beaf23934840b6070a8592dd6d678992d6943
Reviewed-on: https://chromium-review.googlesource.com/c/1436198
Auto-Submit: Gabriel Charette <gab@chromium.org>
Reviewed-by: default avatarDaniel Cheng <dcheng@chromium.org>
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#626223}
parent 561a644d
...@@ -7,7 +7,13 @@ ...@@ -7,7 +7,13 @@
#include <string> #include <string>
#include <utility> #include <utility>
#include "base/atomicops.h"
#include "base/barrier_closure.h"
#include "base/bind.h"
#include "base/logging.h" #include "base/logging.h"
#include "base/system/sys_info.h"
#include "base/threading/platform_thread.h"
#include "base/threading/simple_thread.h"
#include "build/build_config.h" #include "build/build_config.h"
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
...@@ -73,4 +79,118 @@ TEST(NoDestructorTest, InitializerList) { ...@@ -73,4 +79,118 @@ TEST(NoDestructorTest, InitializerList) {
#endif #endif
} // namespace } // namespace
namespace {
// A class whose constructor busy-loops until it is told to complete
// construction.
class BlockingConstructor {
public:
BlockingConstructor() {
EXPECT_FALSE(WasConstructorCalled());
subtle::NoBarrier_Store(&constructor_called_, 1);
EXPECT_TRUE(WasConstructorCalled());
while (!subtle::NoBarrier_Load(&complete_construction_))
PlatformThread::YieldCurrentThread();
done_construction_ = true;
}
~BlockingConstructor() = delete;
// Returns true if BlockingConstructor() was entered.
static bool WasConstructorCalled() {
return subtle::NoBarrier_Load(&constructor_called_);
}
// Instructs BlockingConstructor() that it may now unblock its construction.
static void CompleteConstructionNow() {
subtle::NoBarrier_Store(&complete_construction_, 1);
}
bool done_construction() { return done_construction_; }
private:
// Use Atomic32 instead of AtomicFlag for them to be trivially initialized.
static subtle::Atomic32 constructor_called_;
static subtle::Atomic32 complete_construction_;
bool done_construction_ = false;
DISALLOW_COPY_AND_ASSIGN(BlockingConstructor);
};
// static
subtle::Atomic32 BlockingConstructor::constructor_called_ = 0;
// static
subtle::Atomic32 BlockingConstructor::complete_construction_ = 0;
// A SimpleThread running at |thread_priority| which invokes |before_get|
// (optional) and then invokes thread-safe
// scoped-static-initializationconstruction on its NoDestructor instance.
class BlockingConstructorThread : public SimpleThread {
public:
BlockingConstructorThread(ThreadPriority thread_priority,
OnceClosure before_get)
: SimpleThread("BlockingConstructorThread", Options(thread_priority)),
before_get_(std::move(before_get)) {}
void Run() override {
if (before_get_)
std::move(before_get_).Run();
static NoDestructor<BlockingConstructor> instance;
EXPECT_TRUE(instance->done_construction());
}
private:
OnceClosure before_get_;
DISALLOW_COPY_AND_ASSIGN(BlockingConstructorThread);
};
} // namespace
// Tests that if the thread assigned to construct the local-static
// initialization of the NoDestructor runs at background priority : the
// foreground threads will yield to it enough for it to eventually complete
// construction. While local-static thread-safe initialization isn't specific to
// NoDestructor, it is tested here as NoDestructor is set to replace
// LazyInstance and this is an important regression test for it
// (https://crbug.com/797129).
TEST(NoDestructorTest, PriorityInversionAtStaticInitializationResolves) {
TimeTicks test_begin = TimeTicks::Now();
// Construct BlockingConstructor from a background thread.
BlockingConstructorThread background_getter(ThreadPriority::BACKGROUND,
OnceClosure());
background_getter.Start();
while (!BlockingConstructor::WasConstructorCalled())
PlatformThread::Sleep(TimeDelta::FromMilliseconds(1));
// Spin 4 foreground thread per core contending to get the already under
// construction NoDestructor. When they are all running and poking at it :
// allow the background thread to complete its work.
const int kNumForegroundThreads = 4 * SysInfo::NumberOfProcessors();
std::vector<std::unique_ptr<SimpleThread>> foreground_threads;
RepeatingClosure foreground_thread_ready_callback =
BarrierClosure(kNumForegroundThreads,
BindOnce(&BlockingConstructor::CompleteConstructionNow));
for (int i = 0; i < kNumForegroundThreads; ++i) {
foreground_threads.push_back(std::make_unique<BlockingConstructorThread>(
ThreadPriority::NORMAL, foreground_thread_ready_callback));
foreground_threads.back()->Start();
}
// This test will hang if the foreground threads become stuck in
// NoDestructor's construction per the background thread never being scheduled
// to complete construction.
for (auto& foreground_thread : foreground_threads)
foreground_thread->Join();
background_getter.Join();
// Fail if this test takes more than 5 seconds (it takes 5-10 seconds on a
// Z840 without r527445 but is expected to be fast (~30ms) with the fix).
EXPECT_LT(TimeTicks::Now() - test_begin, TimeDelta::FromSeconds(5));
}
} // namespace base } // namespace base
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