tor-browser

The Tor Browser
git clone https://git.dasho.dev/tor-browser.git
Log | Files | Refs | README | LICENSE

commit d10c3fcd80671731ce53035be193588f7a86c99c
parent 7ed026412d5520328d66978177d7e8d300427e40
Author: Rob Wu <rob@robwu.nl>
Date:   Tue, 30 Sep 2025 23:56:24 +0000

Bug 1982170 - Unit test to verify threadsafe access via nsITimerManager::GetTimers r=xpcom-reviewers,emilio

nsITimerManager::GetTimers() calls nsTimerImpl::GetName(), which used to
not be threadsafe when the callback target is on a different thread.
This was fixed in patch 1 of bug 1979013, by eagerly fetching the name
at the initialization of the timer.

This patch provides the test coverage to serve as a regression test.

Differential Revision: https://phabricator.services.mozilla.com/D260612

Diffstat:
Axpcom/tests/unit/data/test_timers.worker.js | 14++++++++++++++
Mxpcom/tests/unit/test_getTimers.js | 76+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-------------
2 files changed, 77 insertions(+), 13 deletions(-)

diff --git a/xpcom/tests/unit/data/test_timers.worker.js b/xpcom/tests/unit/data/test_timers.worker.js @@ -0,0 +1,14 @@ +"use strict"; + +self.onmessage = e => { + const [cmd, ...args] = e.data; + let returnValue; + if (cmd === "do_call_setTimeout") { + returnValue = setTimeout(() => console.log("Timeout ran...?"), args[0]); + } else if (cmd === "do_call_clearTimeout") { + clearTimeout(args[0]); + } else { + throw new Error(`Unexpected message: ${uneval(e.data)}`); + } + self.postMessage({ replyToCmd: cmd, returnValue }); +}; diff --git a/xpcom/tests/unit/test_getTimers.js b/xpcom/tests/unit/test_getTimers.js @@ -26,6 +26,10 @@ function newTimer(name, delay, type) { const ignoredTimers = [ "BackgroundHangThread_timer", // BHR is Nightly-only, just ignore it. "CCGCScheduler::EnsureGCRunner", // GC runner can be scheduled anytime, randomly. + "IdleMemoryCleanupWantsLaterCheck", // When Worker is used, nsThread::ProcessNextEvent may call MayScheduleIdleMemoryCleanup. + "dom::IdleGCTimerCallback", // When Worker is used. + "dom::PeriodicGCTimerCallback", // When Worker is used. + "IdleRunnableWrapper::SetTimer", // When Worker is used. ]; if (AppConstants.platform == "win") { // On Windows there's a 3min timer added at startup to then add an @@ -50,19 +54,21 @@ function getTimers() { .filter(t => !ignoredTimers.includes(t.name.replace(/\[.*/, ""))); } -function run_test() { - { - let timers = getTimers(); - for (let timer of timers) { - // Print info about unexpected startup timers to help debugging. - info(`${timer.name}: ${timer.delay}ms, ${timer.type}`); - } - Assert.equal( - timers.length, - 0, - "there should be no timer at xpcshell startup" - ); +function getTimersVerbose() { + let timers = getTimers(); + for (let timer of timers) { + // To help with debugging in case of unexpected timers. + info(`${timer.name}: ${timer.delay}ms, ${timer.type}`); } + return timers; +} + +add_task(function test_nsITimerManager_getTimers() { + Assert.equal( + getTimersVerbose().length, + 0, + "there should be no timer at xpcshell startup" + ); let timerData = [ ["t1", 500, Ci.nsITimer.TYPE_ONE_SHOT], @@ -96,4 +102,48 @@ function run_test() { timers.pop().cancel(); } Assert.equal(getTimers().length, 0, "no timer left after cancelling"); -} +}); + +add_task(async function test_getTimers_with_active_worker_thread() { + let worker = new ChromeWorker("resource://test/data/test_timers.worker.js"); + + async function queryWorker(cmd, ...args) { + return new Promise(resolve => { + // One message at a time; we are not going to send multiple messages. + worker.addEventListener( + "message", + e => { + Assert.equal(e.data.replyToCmd, cmd, "Got reply from worker"); + resolve(e.data.returnValue); + }, + { once: true } + ); + worker.postMessage([cmd, ...args]); + }); + } + + // A very long timeout that will certainly outlast the test execution. + const VERY_LONG_TIMEOUT_MS = 3_600_000; + + Assert.equal(getTimersVerbose().length, 0, "no timers before"); + const handle = await queryWorker("do_call_setTimeout", VERY_LONG_TIMEOUT_MS); + + let timers = getTimersVerbose(); + Assert.equal(timers.length, 1, "one timer after?"); + Assert.equal(timers[0].name, "TimeoutExecutor Runnable", "Got timer name"); + + // Although we created a timer with 3600000 delay, it is reported as 3599999. + // As a sanity check, just check that it is close enough. A drift of more + // than 1 hour would be quite unexpected. + Assert.less( + Math.abs(VERY_LONG_TIMEOUT_MS - timers[0].delay), + 60_000, + `Should match the expected timer delay: ${timers[0].delay}` + ); + Assert.equal(Ci.nsITimer.TYPE_ONE_SHOT, timers[0].type, "Got timer type"); + + await queryWorker("do_call_clearTimeout", handle); + + Assert.equal(getTimersVerbose().length, 0, "no timer left after cancelling"); + worker.terminate(); +});