Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Commit 3610cc9

Browse files
authored
[Impeller] Support static thread safety analysis with condition variables. (#47763)
Fixes flutter/flutter#134843
1 parent 111808f commit 3610cc9

File tree

2 files changed

+204
-0
lines changed

2 files changed

+204
-0
lines changed

impeller/base/base_unittests.cc

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,5 +76,72 @@ TEST(StringsTest, CanSPrintF) {
7676
ASSERT_EQ(SPrintF("%sx%.2f", "Hello", 12.122222), "Hellox12.12");
7777
}
7878

79+
struct CVTest {
80+
Mutex mutex;
81+
ConditionVariable cv;
82+
uint32_t rando_ivar IPLR_GUARDED_BY(mutex) = 0;
83+
};
84+
85+
TEST(ConditionVariableTest, WaitUntil) {
86+
CVTest test;
87+
// test.rando_ivar = 12; // <--- Static analysis error
88+
for (size_t i = 0; i < 2; ++i) {
89+
test.mutex.Lock(); // <--- Static analysis error without this.
90+
auto result = test.cv.WaitUntil(
91+
test.mutex,
92+
std::chrono::high_resolution_clock::now() +
93+
std::chrono::milliseconds{10},
94+
[&]() IPLR_REQUIRES(test.mutex) {
95+
test.rando_ivar = 12; // <-- Static analysics error without the
96+
// IPLR_REQUIRES on the pred.
97+
return false;
98+
});
99+
test.mutex.Unlock();
100+
ASSERT_FALSE(result);
101+
}
102+
Lock lock(test.mutex); // <--- Static analysis error without this.
103+
// The predicate never returns true. So return has to be due to a non-spurious
104+
// wake.
105+
ASSERT_EQ(test.rando_ivar, 12u);
106+
}
107+
108+
TEST(ConditionVariableTest, WaitFor) {
109+
CVTest test;
110+
// test.rando_ivar = 12; // <--- Static analysis error
111+
for (size_t i = 0; i < 2; ++i) {
112+
test.mutex.Lock(); // <--- Static analysis error without this.
113+
auto result = test.cv.WaitFor(
114+
test.mutex, std::chrono::milliseconds{10},
115+
[&]() IPLR_REQUIRES(test.mutex) {
116+
test.rando_ivar = 12; // <-- Static analysics error without the
117+
// IPLR_REQUIRES on the pred.
118+
return false;
119+
});
120+
test.mutex.Unlock();
121+
ASSERT_FALSE(result);
122+
}
123+
Lock lock(test.mutex); // <--- Static analysis error without this.
124+
// The predicate never returns true. So return has to be due to a non-spurious
125+
// wake.
126+
ASSERT_EQ(test.rando_ivar, 12u);
127+
}
128+
129+
TEST(ConditionVariableTest, WaitForever) {
130+
CVTest test;
131+
// test.rando_ivar = 12; // <--- Static analysis error
132+
for (size_t i = 0; i < 2; ++i) {
133+
test.mutex.Lock(); // <--- Static analysis error without this.
134+
test.cv.Wait(test.mutex, [&]() IPLR_REQUIRES(test.mutex) {
135+
test.rando_ivar = 12; // <-- Static analysics error without
136+
// the IPLR_REQUIRES on the pred.
137+
return true;
138+
});
139+
test.mutex.Unlock();
140+
}
141+
Lock lock(test.mutex); // <--- Static analysis error without this.
142+
// The wake only happens when the predicate returns true.
143+
ASSERT_EQ(test.rando_ivar, 12u);
144+
}
145+
79146
} // namespace testing
80147
} // namespace impeller

impeller/base/thread.h

Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,9 @@
44

55
#pragma once
66

7+
#include <chrono>
8+
#include <condition_variable>
9+
#include <functional>
710
#include <memory>
811
#include <mutex>
912
#include <thread>
@@ -14,6 +17,8 @@
1417

1518
namespace impeller {
1619

20+
class ConditionVariable;
21+
1722
class IPLR_CAPABILITY("mutex") Mutex {
1823
public:
1924
Mutex() = default;
@@ -25,6 +30,8 @@ class IPLR_CAPABILITY("mutex") Mutex {
2530
void Unlock() IPLR_RELEASE() { mutex_.unlock(); }
2631

2732
private:
33+
friend class ConditionVariable;
34+
2835
std::mutex mutex_;
2936

3037
Mutex(const Mutex&) = delete;
@@ -124,4 +131,134 @@ class IPLR_SCOPED_CAPABILITY WriterLock {
124131
WriterLock& operator=(WriterLock&&) = delete;
125132
};
126133

134+
//------------------------------------------------------------------------------
135+
/// @brief A condition variable exactly similar to the one in libcxx with
136+
/// two major differences:
137+
///
138+
/// * On the Wait, WaitFor, and WaitUntil calls, static analysis
139+
/// annotation are respected.
140+
/// * There is no ability to wait on a condition variable and also
141+
/// be susceptible to spurious wakes. This is because the
142+
/// predicate is mandatory.
143+
///
144+
class ConditionVariable {
145+
public:
146+
ConditionVariable() = default;
147+
148+
~ConditionVariable() = default;
149+
150+
ConditionVariable(const ConditionVariable&) = delete;
151+
152+
ConditionVariable& operator=(const ConditionVariable&) = delete;
153+
154+
void NotifyOne() { cv_.notify_one(); }
155+
156+
void NotifyAll() { cv_.notify_all(); }
157+
158+
using Predicate = std::function<bool()>;
159+
160+
//----------------------------------------------------------------------------
161+
/// @brief Atomically unlocks the mutex and waits on the condition
162+
/// variable up to a specified time point. Spurious wakes may
163+
/// happen before the time point is reached. In such cases the
164+
/// predicate is invoked and it must return `false` for the wait
165+
/// to continue. The predicate will be invoked with the mutex
166+
/// locked.
167+
///
168+
/// @note Since the predicate is invoked with the mutex locked, if it
169+
/// accesses other guarded resources, the predicate itself must be
170+
/// decorated with the IPLR_REQUIRES directive. For instance,
171+
///
172+
/// ```c++
173+
/// [] () IPLR_REQUIRES(mutex) {
174+
/// return my_guarded_resource.should_stop_waiting;
175+
/// }
176+
/// ```
177+
///
178+
/// @param mutex The mutex.
179+
/// @param[in] time_point The time point to wait to.
180+
/// @param[in] should_stop_waiting The predicate invoked on spurious wakes.
181+
/// Must return false for the wait to
182+
/// continue.
183+
///
184+
/// @tparam Clock The clock type.
185+
/// @tparam Duration The duration type.
186+
///
187+
/// @return The value of the predicate at the end of the wait.
188+
///
189+
template <class Clock, class Duration>
190+
bool WaitUntil(Mutex& mutex,
191+
const std::chrono::time_point<Clock, Duration>& time_point,
192+
const Predicate& should_stop_waiting) IPLR_REQUIRES(mutex) {
193+
std::unique_lock lock(mutex.mutex_, std::adopt_lock);
194+
return cv_.wait_until(lock, time_point, should_stop_waiting);
195+
}
196+
197+
//----------------------------------------------------------------------------
198+
/// @brief Atomically unlocks the mutex and waits on the condition
199+
/// variable for a designated duration. Spurious wakes may happen
200+
/// before the time point is reached. In such cases the predicate
201+
/// is invoked and it must return `false` for the wait to
202+
/// continue. The predicate will be invoked with the mutex locked.
203+
///
204+
/// @note Since the predicate is invoked with the mutex locked, if it
205+
/// accesses other guarded resources, the predicate itself must be
206+
/// decorated with the IPLR_REQUIRES directive. For instance,
207+
///
208+
/// ```c++
209+
/// [] () IPLR_REQUIRES(mutex) {
210+
/// return my_guarded_resource.should_stop_waiting;
211+
/// }
212+
/// ```
213+
///
214+
/// @param mutex The mutex.
215+
/// @param[in] duration The duration to wait for.
216+
/// @param[in] should_stop_waiting The predicate invoked on spurious wakes.
217+
/// Must return false for the wait to
218+
/// continue.
219+
///
220+
/// @tparam Representation The duration representation type.
221+
/// @tparam Period The duration period type.
222+
///
223+
/// @return The value of the predicate at the end of the wait.
224+
///
225+
template <class Representation, class Period>
226+
bool WaitFor(Mutex& mutex,
227+
const std::chrono::duration<Representation, Period>& duration,
228+
const Predicate& should_stop_waiting) IPLR_REQUIRES(mutex) {
229+
return WaitUntil(mutex, std::chrono::steady_clock::now() + duration,
230+
should_stop_waiting);
231+
}
232+
233+
//----------------------------------------------------------------------------
234+
/// @brief Atomically unlocks the mutex and waits on the condition
235+
/// variable indefinitely till the predicate determines that the
236+
/// wait must end. Spurious wakes may happen before the time point
237+
/// is reached. In such cases the predicate is invoked and it must
238+
/// return `false` for the wait to continue. The predicate will be
239+
/// invoked with the mutex locked.
240+
///
241+
/// @note Since the predicate is invoked with the mutex locked, if it
242+
/// accesses other guarded resources, the predicate itself must be
243+
/// decorated with the IPLR_REQUIRES directive. For instance,
244+
///
245+
/// ```c++
246+
/// [] () IPLR_REQUIRES(mutex) {
247+
/// return my_guarded_resource.should_stop_waiting;
248+
/// }
249+
/// ```
250+
///
251+
/// @param mutex The mutex
252+
/// @param[in] should_stop_waiting The should stop waiting
253+
///
254+
void Wait(Mutex& mutex, const Predicate& should_stop_waiting)
255+
IPLR_REQUIRES(mutex) {
256+
std::unique_lock lock(mutex.mutex_, std::adopt_lock);
257+
cv_.wait(lock, should_stop_waiting);
258+
}
259+
260+
private:
261+
std::condition_variable cv_;
262+
};
263+
127264
} // namespace impeller

0 commit comments

Comments
 (0)