Elly Fong-Jones | 56f11aa7 | 2020-04-03 14:14:48 | [diff] [blame] | 1 | # The Synchronous RunLoop Pattern |
| 2 | |
| 3 | The synchronous RunLoop pattern involves creating a new RunLoop, setting up a |
| 4 | specified quit condition for it, then calling Run() on it to block the current |
| 5 | thread until that quit condition is reached. |
| 6 | |
| 7 | ## Use this pattern when: |
| 8 | |
| 9 | You need to **block the current thread** until an event happens, and you have a |
| 10 | way to get notified of that event, via a callback or observer interface or |
| 11 | similar. A couple of common scenarios might be: |
| 12 | |
| 13 | * Waiting for an asynchronous event (like a network request) to complete |
| 14 | * Waiting for an animation to finish |
| 15 | * Waiting for a page to have loaded |
| 16 | * Waiting for some call that requires a thread hop to complete |
| 17 | |
| 18 | The fact that this blocks a thread means it is **almost never appropriate |
| 19 | outside test code**. |
| 20 | |
| 21 | ## Don't use this pattern when: |
| 22 | |
| 23 | * You don't really need the entire thread to wait |
| 24 | * You don't have and can't add a way to get notified when the event happens |
| 25 | * You're waiting for a timer to fire - for that, [TaskEnvironment] is likely a |
| 26 | better fit. |
| 27 | |
| 28 | ## Alternatives / see also: |
| 29 | |
| 30 | * [TaskEnvironment] |
| 31 | * Restructuring your code to not require blocking a thread |
| 32 | |
| 33 | ## How this pattern works: |
| 34 | |
| 35 | This pattern relies on two important facts about [base::RunLoop]: |
| 36 | |
| 37 | 1. `base::RunLoop::Quit()` is idempotent - once a RunLoop enters the quit |
| 38 | state, quitting it again does nothing |
| 39 | 2. Once a RunLoop is in the quit state, calling `base::RunLoop::Run()` on it is |
| 40 | a no-op |
| 41 | |
| 42 | That means that if your code does this: |
| 43 | |
| 44 | ```c++ |
Jan Wilken Dörrie | d02749a | 2020-10-26 15:51:03 | [diff] [blame] | 45 | base::RunLoop loop; |
| 46 | maybe-asynchronously { loop.Quit(); } |
| 47 | loop.Run(); |
| 48 | LOG(INFO) << "Hello!"; |
Elly Fong-Jones | 56f11aa7 | 2020-04-03 14:14:48 | [diff] [blame] | 49 | ``` |
| 50 | |
| 51 | then regardless of whether the maybe-asynchronous `loop.Quit()` is executed |
| 52 | before or after `loop.Run()`, the "Hello!" message will never be printed before |
| 53 | both `loop.Run()` and `loop.Quit()` have happened. If the `Quit` happens |
| 54 | before the `Run`, the `Run` will be a no-op; if the `Quit` happens after the |
| 55 | `Run` has started, the `Run` will exit after the `Quit`. |
| 56 | |
| 57 | ## How to use this pattern in Chromium: |
| 58 | |
| 59 | If the asynchronous thing in question takes a completion callback: |
| 60 | |
| 61 | ```c++ |
Jan Wilken Dörrie | d02749a | 2020-10-26 15:51:03 | [diff] [blame] | 62 | base::RunLoop run_loop; |
| 63 | Reply reply; |
| 64 | DoThingAndReply( |
| 65 | base::BindLambdaForTesting([&](const Reply& r) { |
| 66 | reply = r; |
| 67 | run_loop.Quit(); |
| 68 | })); |
| 69 | run_loop.Run(); |
Elly Fong-Jones | 56f11aa7 | 2020-04-03 14:14:48 | [diff] [blame] | 70 | ``` |
| 71 | |
| 72 | or perhaps even just: |
| 73 | |
| 74 | ```c++ |
Jan Wilken Dörrie | d02749a | 2020-10-26 15:51:03 | [diff] [blame] | 75 | base::RunLoop run_loop; |
| 76 | DoThing(run_loop.QuitClosure()); |
| 77 | run_loop.Run(); |
Elly Fong-Jones | 56f11aa7 | 2020-04-03 14:14:48 | [diff] [blame] | 78 | ``` |
| 79 | |
| 80 | If there exists a GizmoObserver interface with an OnThingDone event: |
| 81 | |
| 82 | ```c++ |
Jan Wilken Dörrie | d02749a | 2020-10-26 15:51:03 | [diff] [blame] | 83 | class TestGizmoObserver : public GizmoObserver { |
| 84 | public: |
| 85 | TestGizmoObserver(base::RunLoop* loop, Gizmo* thing) |
| 86 | : GizmoObserver(thing), loop_(loop) {} |
Elly Fong-Jones | 56f11aa7 | 2020-04-03 14:14:48 | [diff] [blame] | 87 | |
Jan Wilken Dörrie | d02749a | 2020-10-26 15:51:03 | [diff] [blame] | 88 | // GizmoObserver: |
| 89 | void OnThingStarted(Gizmo* observed_gizmo) override { ... } |
| 90 | void OnThingProgressed(Gizmo* observed_gizmo) override { ... } |
| 91 | void OnThingDone(Gizmo* observed_gizmo) override { |
| 92 | loop_->Quit(); |
| 93 | } |
| 94 | }; |
Elly Fong-Jones | 56f11aa7 | 2020-04-03 14:14:48 | [diff] [blame] | 95 | |
Jan Wilken Dörrie | d02749a | 2020-10-26 15:51:03 | [diff] [blame] | 96 | base::RunLoop run_loop; |
| 97 | TestGizmoObserver observer(&run_loop, gizmo); |
| 98 | gizmo->StartDoingThing(); |
| 99 | run_loop.Run(); |
Elly Fong-Jones | 56f11aa7 | 2020-04-03 14:14:48 | [diff] [blame] | 100 | ``` |
| 101 | |
| 102 | This is sometimes wrapped up into a helper class that internally constructs the |
| 103 | RunLoop like so, if all you need to do is wait for the event but don't care |
| 104 | about observing any intermediate states too: |
| 105 | |
| 106 | ```c++ |
Jan Wilken Dörrie | d02749a | 2020-10-26 15:51:03 | [diff] [blame] | 107 | class ThingDoneWaiter : public GizmoObserver { |
| 108 | public: |
| 109 | ThingDoneWaiter(Gizmo* thing) : GizmoObserver(thing) {} |
Elly Fong-Jones | 56f11aa7 | 2020-04-03 14:14:48 | [diff] [blame] | 110 | |
Jan Wilken Dörrie | d02749a | 2020-10-26 15:51:03 | [diff] [blame] | 111 | void Wait() { |
| 112 | run_loop_.Run(); |
| 113 | } |
Elly Fong-Jones | 56f11aa7 | 2020-04-03 14:14:48 | [diff] [blame] | 114 | |
Jan Wilken Dörrie | d02749a | 2020-10-26 15:51:03 | [diff] [blame] | 115 | // GizmoObserver: |
| 116 | void OnThingDone(Gizmo* observed_gizmo) { |
| 117 | run_loop_.Quit(); |
| 118 | } |
Elly Fong-Jones | 56f11aa7 | 2020-04-03 14:14:48 | [diff] [blame] | 119 | |
Jan Wilken Dörrie | d02749a | 2020-10-26 15:51:03 | [diff] [blame] | 120 | private: |
| 121 | RunLoop run_loop_; |
| 122 | }; |
Elly Fong-Jones | 56f11aa7 | 2020-04-03 14:14:48 | [diff] [blame] | 123 | |
Jan Wilken Dörrie | d02749a | 2020-10-26 15:51:03 | [diff] [blame] | 124 | ThingDoneWaiter waiter(gizmo); |
| 125 | gizmo->StartDoingThing(); |
| 126 | waiter.Wait(); |
Elly Fong-Jones | 56f11aa7 | 2020-04-03 14:14:48 | [diff] [blame] | 127 | ``` |
| 128 | |
Devlin Cronin | 759ba8a | 2020-04-16 16:57:06 | [diff] [blame] | 129 | ## Events vs States |
| 130 | |
| 131 | It's important to differentiate between waiting on an *event* (such as a |
| 132 | notification or callback being fired) vs waiting for a *state* (such as a |
| 133 | property on a given object). |
| 134 | |
| 135 | When waiting for events, it is crucial that the observer is constructed in time |
Frédéric Wang | 796f4db | 2020-08-21 08:23:21 | [diff] [blame] | 136 | to see the event (see also [waiting too late](#starting-to-wait-for-an-event-too-late)). |
Devlin Cronin | 759ba8a | 2020-04-16 16:57:06 | [diff] [blame] | 137 | States, on the other hand, can be queried beforehand in the body of a |
| 138 | Wait()-style function. |
| 139 | |
| 140 | The following is an example of a Waiter helper class that waits for a state, as |
| 141 | opposed to an event: |
| 142 | |
| 143 | ```c++ |
Jan Wilken Dörrie | d02749a | 2020-10-26 15:51:03 | [diff] [blame] | 144 | class GizmoReadyWaiter : public GizmoObserver { |
| 145 | public: |
| 146 | GizmoReadyObserver(Gizmo* gizmo) |
| 147 | : gizmo_(gizmo) {} |
| 148 | ~GizmoReadyObserver() override = default; |
Devlin Cronin | 759ba8a | 2020-04-16 16:57:06 | [diff] [blame] | 149 | |
Jan Wilken Dörrie | d02749a | 2020-10-26 15:51:03 | [diff] [blame] | 150 | void WaitForGizmoReady() { |
| 151 | if (!gizmo_->ready()) { |
Sigurdur Asgeirsson | fb9a9f7 | 2021-05-20 20:45:17 | [diff] [blame] | 152 | gizmo_observation_.Observe(gizmo_); |
Jan Wilken Dörrie | d02749a | 2020-10-26 15:51:03 | [diff] [blame] | 153 | run_loop_.Run(); |
Devlin Cronin | 759ba8a | 2020-04-16 16:57:06 | [diff] [blame] | 154 | } |
Jan Wilken Dörrie | d02749a | 2020-10-26 15:51:03 | [diff] [blame] | 155 | } |
Devlin Cronin | 759ba8a | 2020-04-16 16:57:06 | [diff] [blame] | 156 | |
Jan Wilken Dörrie | d02749a | 2020-10-26 15:51:03 | [diff] [blame] | 157 | // GizmoObserver: |
| 158 | void OnGizmoReady(Gizmo* observed_gizmo) { |
| 159 | run_loop_.Quit(); |
| 160 | } |
Devlin Cronin | 759ba8a | 2020-04-16 16:57:06 | [diff] [blame] | 161 | |
Jan Wilken Dörrie | d02749a | 2020-10-26 15:51:03 | [diff] [blame] | 162 | private: |
| 163 | RunLoop run_loop_; |
| 164 | Gizmo* gizmo_; |
Sigurdur Asgeirsson | fb9a9f7 | 2021-05-20 20:45:17 | [diff] [blame] | 165 | base::ScopedObservation<Gizmo, GizmoObserver> gizmo_observation_{this}; |
Jan Wilken Dörrie | d02749a | 2020-10-26 15:51:03 | [diff] [blame] | 166 | }; |
Devlin Cronin | 759ba8a | 2020-04-16 16:57:06 | [diff] [blame] | 167 | ``` |
| 168 | |
Elly Fong-Jones | 56f11aa7 | 2020-04-03 14:14:48 | [diff] [blame] | 169 | ## Sharp edges |
| 170 | |
Devlin Cronin | 759ba8a | 2020-04-16 16:57:06 | [diff] [blame] | 171 | ### Starting to wait for an event too late |
Elly Fong-Jones | 56f11aa7 | 2020-04-03 14:14:48 | [diff] [blame] | 172 | |
| 173 | A common mis-use of this pattern is like so: |
| 174 | |
| 175 | ```c++ |
Jan Wilken Dörrie | d02749a | 2020-10-26 15:51:03 | [diff] [blame] | 176 | gizmo->StartDoingThing(); |
| 177 | base::RunLoop run_loop; |
| 178 | TestGizmoObserver observer(&run_loop, gizmo); |
| 179 | run_loop.Run(); |
Elly Fong-Jones | 56f11aa7 | 2020-04-03 14:14:48 | [diff] [blame] | 180 | ``` |
| 181 | |
| 182 | This looks tempting because it seems that you can write a helper function: |
| 183 | |
| 184 | ```c++ |
Jan Wilken Dörrie | d02749a | 2020-10-26 15:51:03 | [diff] [blame] | 185 | void TerribleHorribleNoGoodVeryBadWaitForThing(Gizmo* gizmo) { |
| 186 | base::RunLoop run_loop; |
| 187 | TestGizmoObserver observer(&run_loop, gizmo); |
| 188 | run_loop.Run(); |
| 189 | } |
Elly Fong-Jones | 56f11aa7 | 2020-04-03 14:14:48 | [diff] [blame] | 190 | ``` |
| 191 | |
| 192 | and then your test code can simply read: |
| 193 | |
| 194 | ```c++ |
Jan Wilken Dörrie | d02749a | 2020-10-26 15:51:03 | [diff] [blame] | 195 | gizmo->StartDoingThing(); |
| 196 | TerribleHorribleNoGoodVeryBadWaitForThing(gizmo); |
Elly Fong-Jones | 56f11aa7 | 2020-04-03 14:14:48 | [diff] [blame] | 197 | ``` |
| 198 | |
| 199 | However, this is a recipe for a flaky test: if `gizmo->StartDoingThing()` |
| 200 | *completes* and would deliver the `OnThingDone` callback before your |
| 201 | `TestGizmoObserver` is ever constructed, the `TestGizmoObserver` will never |
| 202 | receive `OnThingDone`, and then your `run_loop.Run()` will run forever, |
| 203 | frustrating a future tree sheriff (and then probably you, shortly afterward). |
| 204 | This is especially dangerous when `gizmo->StartDoingThing()` involves a thread |
| 205 | hop or network request, because these can unpredictably complete before or after |
| 206 | your observer gets constructed. To be safe, always begin observing the event |
| 207 | *before* running the code that will eventually cause the event! |
| 208 | |
| 209 | If you still really want a helper function, perhaps you just want to inline the |
| 210 | start: |
| 211 | |
| 212 | ```c++ |
Jan Wilken Dörrie | d02749a | 2020-10-26 15:51:03 | [diff] [blame] | 213 | void NiceFriendlyDoThingAndWait(Gizmo* gizmo) { |
| 214 | base::RunLoop run_loop; |
| 215 | TestGizmoObserver observer(&run_loop, gizmo); |
| 216 | gizmo->StartDoingThing(); |
| 217 | run_loop.Run(); |
| 218 | } |
Elly Fong-Jones | 56f11aa7 | 2020-04-03 14:14:48 | [diff] [blame] | 219 | ``` |
| 220 | |
| 221 | with the test code being: |
| 222 | |
| 223 | ```c++ |
Jan Wilken Dörrie | d02749a | 2020-10-26 15:51:03 | [diff] [blame] | 224 | NiceFriendlyDoThingAndWait(gizmo); |
Elly Fong-Jones | 56f11aa7 | 2020-04-03 14:14:48 | [diff] [blame] | 225 | ``` |
| 226 | |
Devlin Cronin | 759ba8a | 2020-04-16 16:57:06 | [diff] [blame] | 227 | Note that this is not an issue when waiting on a *state*, since the observer can |
| 228 | query to see if that state is already the current state. |
| 229 | |
Elly Fong-Jones | 56f11aa7 | 2020-04-03 14:14:48 | [diff] [blame] | 230 | ### Guessing RunLoop cycles |
| 231 | |
| 232 | Sometimes, there's no easy way to observe completion of an event. In that case, |
| 233 | if the code under test looks like this: |
| 234 | |
| 235 | ```c++ |
Jan Wilken Dörrie | d02749a | 2020-10-26 15:51:03 | [diff] [blame] | 236 | void StartDoingThing() { PostTask(&StepOne); } |
| 237 | void StepOne() { PostTask(&StepTwo); } |
| 238 | void StepTwo() { /* done! */ } |
Elly Fong-Jones | 56f11aa7 | 2020-04-03 14:14:48 | [diff] [blame] | 239 | ``` |
| 240 | |
| 241 | it can be tempting to do: |
| 242 | |
| 243 | ```c++ |
Jan Wilken Dörrie | d02749a | 2020-10-26 15:51:03 | [diff] [blame] | 244 | gizmo->StartDoingThing(); |
| 245 | base::RunLoop().RunUntilIdle(); |
| 246 | /* now it's done! */ |
Elly Fong-Jones | 56f11aa7 | 2020-04-03 14:14:48 | [diff] [blame] | 247 | ``` |
| 248 | |
| 249 | However, doing this is adding dependencies to your test code on the exact async |
| 250 | behavior of the production code - for example, the production code may depend on |
| 251 | work happening on another TaskRunner, which this won't successfully wait for. |
| 252 | This will make your test brittle and flaky. |
| 253 | |
| 254 | Instead of doing this, it's vastly better to add a way (even if it's just via a |
| 255 | [test API]) to observe the event you're interested in. |
| 256 | |
Devlin Cronin | aeb91b36 | 2020-04-16 18:48:40 | [diff] [blame] | 257 | ### Not managing lifetimes |
| 258 | |
| 259 | As with most patterns, lifetimes can be an issue with this pattern when using |
| 260 | observers. If you are waiting on a given event to happen, and the object that's |
| 261 | being observed instead goes out of scope, the test may hang. |
| 262 | Similar badness can happen if the Waiter isn't properly removed as an observer, |
| 263 | which could lead to Use-After-Frees. |
| 264 | |
| 265 | There are two good mitigation practices here. |
| 266 | |
| 267 | #### Keep Waiter-style helper classes as narrowly scoped as possible. |
| 268 | Consider something like |
| 269 | ```c++ |
| 270 | TEST_F(GizmoTest, WaitForGizmo) { |
| 271 | GizmoWaiter waiter; |
| 272 | Gizmo gizmo; |
| 273 | gizmo.Initialize(); |
| 274 | waiter.WaitForGizmoReady(); |
| 275 | ASSERT_TRUE(gizmo.ready()); |
| 276 | } |
| 277 | ``` |
| 278 | |
| 279 | This looks safe, but may not be. If GizmoObserver removes itself as an observer |
| 280 | from Gizmo in its destructor, this will result in a Use-After-Free during the |
| 281 | test tear down. Instead, scope the GizmoWaiter more narrowly: |
| 282 | ```c++ |
| 283 | TEST_F(GizmoTest, WaitForGizmo) { |
| 284 | Gizmo gizmo; |
| 285 | { |
| 286 | GizmoWaiter waiter; |
| 287 | gizmo.Initialize(); |
| 288 | waiter.WaitForGizmoReady(); |
| 289 | } |
| 290 | ASSERT_TRUE(gizmo.ready()); |
| 291 | } |
| 292 | ``` |
| 293 | |
| 294 | Since the GizmoWaiter is now narrowly-scoped, it will be destroyed when it is |
| 295 | no longer needed, and avoid Use-After-Free concerns. |
| 296 | |
| 297 | #### If in doubt, handle the destruction case appropriately |
| 298 | If you need to potentially handle the case where the object being observed is |
| 299 | destroyed while a waiter is still active, you can handle the destruction case |
| 300 | gracefully. |
| 301 | |
| 302 | |
| 303 | ```c++ |
Jan Wilken Dörrie | d02749a | 2020-10-26 15:51:03 | [diff] [blame] | 304 | class GizmoReadyWaiter : public GizmoObserver { |
| 305 | public: |
| 306 | GizmoReadyObserver(Gizmo* gizmo) |
| 307 | : gizmo_(gizmo) {} |
| 308 | ~GizmoReadyObserver() override = default; |
Devlin Cronin | aeb91b36 | 2020-04-16 18:48:40 | [diff] [blame] | 309 | |
Jan Wilken Dörrie | d02749a | 2020-10-26 15:51:03 | [diff] [blame] | 310 | void WaitForGizmoReady() { |
| 311 | ASSERT_TRUE(gizmo_) |
| 312 | << "Trying to call Wait() after the Gizmo was destroyed!"; |
| 313 | if (!gizmo_->ready()) { |
Sigurdur Asgeirsson | fb9a9f7 | 2021-05-20 20:45:17 | [diff] [blame] | 314 | gizmo_observation_.Observe(gizmo_); |
Jan Wilken Dörrie | d02749a | 2020-10-26 15:51:03 | [diff] [blame] | 315 | run_loop_.Run(); |
Devlin Cronin | aeb91b36 | 2020-04-16 18:48:40 | [diff] [blame] | 316 | } |
Jan Wilken Dörrie | d02749a | 2020-10-26 15:51:03 | [diff] [blame] | 317 | } |
Devlin Cronin | aeb91b36 | 2020-04-16 18:48:40 | [diff] [blame] | 318 | |
Jan Wilken Dörrie | d02749a | 2020-10-26 15:51:03 | [diff] [blame] | 319 | // GizmoObserver: |
| 320 | void OnGizmoReady(Gizmo* observed_gizmo) { |
Sigurdur Asgeirsson | fb9a9f7 | 2021-05-20 20:45:17 | [diff] [blame] | 321 | gizmo_observation_.Reset(); |
Jan Wilken Dörrie | d02749a | 2020-10-26 15:51:03 | [diff] [blame] | 322 | run_loop_.Quit(); |
| 323 | } |
| 324 | void OnGizmoDestroying(Gizmo* observed_gizmo) { |
| 325 | DCHECK_EQ(gizmo_, observed_gizmo); |
| 326 | gizmo_ = nullptr; |
| 327 | // Remove the observer now, to avoid a UAF in the destructor. |
Sigurdur Asgeirsson | fb9a9f7 | 2021-05-20 20:45:17 | [diff] [blame] | 328 | gizmo_observation_.Reset(); |
Jan Wilken Dörrie | d02749a | 2020-10-26 15:51:03 | [diff] [blame] | 329 | // Bail out so we don't time out in the test waiting for a ready state |
| 330 | // that will never come. |
| 331 | run_loop_.Quit(); |
| 332 | // Was this a possible expected outcome? If not, consider: |
| 333 | // ADD_FAILURE() << "The Gizmo was destroyed before it was ready!"; |
| 334 | } |
Devlin Cronin | aeb91b36 | 2020-04-16 18:48:40 | [diff] [blame] | 335 | |
Jan Wilken Dörrie | d02749a | 2020-10-26 15:51:03 | [diff] [blame] | 336 | private: |
| 337 | RunLoop run_loop_; |
| 338 | Gizmo* gizmo_; |
Sigurdur Asgeirsson | fb9a9f7 | 2021-05-20 20:45:17 | [diff] [blame] | 339 | base::ScopedObservation<Gizmo, GizmoObserver> gizmo_observation_{this}; |
Jan Wilken Dörrie | d02749a | 2020-10-26 15:51:03 | [diff] [blame] | 340 | }; |
Devlin Cronin | aeb91b36 | 2020-04-16 18:48:40 | [diff] [blame] | 341 | ``` |
| 342 | |
Elly Fong-Jones | 56f11aa7 | 2020-04-03 14:14:48 | [diff] [blame] | 343 | [base::RunLoop]: ../../base/run_loop.h |
| 344 | [TaskEnvironment]: ../threading_and_tasks_testing.md |
| 345 | [test API]: testapi.md |