From cef7c4e81afba04c30ead19cc689e337edbf1901 Mon Sep 17 00:00:00 2001 From: Abseil Team Date: Fri, 16 Jun 2023 01:48:02 -0700 Subject: absl: fix Mutex writer starvation related to uninit priority Currently when we queue the first thread, we don't init its priority. Subsequent queued threads init priority, but they compare it against the first thread priority, which is uninit. Thus the order can be wrong. It can lead to complete false starvation in some corner cases. On Linux the default priority is 0, which matches the uninit value, thus the problem is harder to spot on Linux (only possible if explicit thread priorities are used). But on Darwin the default priority is 31, thus the first thread falsely looks like lower priority than subsequently queued threads. The added test exposes the problem on Darwin. Always initialize the priority before queuing threads. PiperOrigin-RevId: 540814133 Change-Id: I513ce1493a67afe77d3e92fb49000b046b42a9f2 --- absl/synchronization/mutex_test.cc | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) (limited to 'absl/synchronization/mutex_test.cc') diff --git a/absl/synchronization/mutex_test.cc b/absl/synchronization/mutex_test.cc index 4ae4d7e7..35802b2e 100644 --- a/absl/synchronization/mutex_test.cc +++ b/absl/synchronization/mutex_test.cc @@ -1838,4 +1838,34 @@ TEST(Mutex, SignalExitedThread) { for (auto &th : top) th.join(); } +TEST(Mutex, WriterPriority) { + absl::Mutex mu; + bool wrote = false; + std::atomic saw_wrote{false}; + auto readfunc = [&]() { + for (size_t i = 0; i < 10; ++i) { + absl::ReaderMutexLock lock(&mu); + if (wrote) { + saw_wrote = true; + break; + } + absl::SleepFor(absl::Seconds(1)); + } + }; + std::thread t1(readfunc); + absl::SleepFor(absl::Milliseconds(500)); + std::thread t2(readfunc); + // Note: this test guards against a bug that was related to an uninit + // PerThreadSynch::priority, so the writer intentionally runs on a new thread. + std::thread t3([&]() { + // The writer should be able squeeze between the two alternating readers. + absl::MutexLock lock(&mu); + wrote = true; + }); + t1.join(); + t2.join(); + t3.join(); + EXPECT_TRUE(saw_wrote.load()); +} + } // namespace -- cgit v1.2.3