Magic-Mirror/gecko-dev
1
0
Fork 0
mirror of https://github.com/mozilla/gecko-dev.git synced 2025-03-04 07:40:42 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Bug 1704887 - Fix a UAF in the DelayedRunnable gtests. r=xpcom-reviewers,nika

Browse Source 
In the gtest for these patches, we can encounter a hang in the following steps:
- Monitor waits in the main thread (for async steps to finish)
- Offthread, monitor waits for the timer to fire
- Timer notifies the waiting monitors
- Monitor on the main thread continues first, wrapping up the test. It assumes all async steps are finished.
- The offthread monitor wait follows, but at this point the main thread has dereferenced the monitor. Because of this race, we run into a UAF and hang on the offthread monitor.

My solution for this is to use two monitors, and notify the outer one after we have=completed all of the async steps including the wait for the timer notification. This should avoid a race between the inner `monitor.wait()` and the free at the end of the tests.

Tentative try push for the fix: https://treeherder.mozilla.org/jobs?repo=try&revision=307960b2899b320ef5a82d276b86d633a9653941

Differential Revision: https://phabricator.services.mozilla.com/D116163
This commit is contained in:
Kris Wright 2021-05-28 19:05:30 +00:00
parent e9dca94b9d
commit 68285a5475
1 changed files with 10 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
xpcom/tests/gtest/TestDelayedRunnable.cpp
View File

@ -113,8 +113,8 @@ TEST(DelayedRunnable, TimerFiresBeforeRunnableRuns)
do_AddRef(pool), /* aSupportsTailDispatch = */ true);
auto noTailTaskQueue = MakeRefPtr<TaskQueue>(
do_AddRef(pool), /* aSupportsTailDispatch = */ false);
Monitor monitor(__func__);
MonitorAutoLock lock(monitor);
Monitor outerMonitor(__func__);
MonitorAutoLock lock(outerMonitor);
MOZ_ALWAYS_SUCCEEDS(
tailTaskQueue1->Dispatch(NS_NewRunnableFunction(__func__, [&] {
// This will tail dispatch the delayed runnable, making it prone to
@ -123,18 +123,21 @@ TEST(DelayedRunnable, TimerFiresBeforeRunnableRuns)
EXPECT_TRUE(tailTaskQueue1->RequiresTailDispatch(tailTaskQueue2));
tailTaskQueue2->DelayedDispatch(
NS_NewRunnableFunction(__func__, [&] {}), 1);
MonitorAutoLock lock(monitor);
Monitor innerMonitor(__func__);
MonitorAutoLock lock(innerMonitor);
auto timer = MakeRefPtr<MediaTimer>();
timer->WaitFor(TimeDuration::FromMilliseconds(1), __func__)
->Then(noTailTaskQueue, __func__, [&] {
MonitorAutoLock lock(monitor);
monitor.NotifyAll();
MonitorAutoLock lock(innerMonitor);
innerMonitor.NotifyAll();
});
// Wait until the timer has run. It should have dispatched the
// TimerEvent to tailTaskQueue2 by then. The tail dispatch happens when
// we leave scope.
monitor.Wait();
innerMonitor.Wait();
// Notify the outer monitor that we've finished the async steps.
outerMonitor.NotifyAll();
})));
// Wait for async steps before wrapping up the test case.
monitor.Wait();
outerMonitor.Wait();
}