Skip to content

Commit b9b3c5a

Browse files
committed
Bug 2050049 - Dump and report a profile when an xpcshell test is about to time out, r=ahal.
Arm the profiler's scheduled off-main-thread dump (scheduleDumpToFile) shortly before the harness timeout fires, so a test wedged in a synchronous run -- where the main-thread nsITimer in head.js could never fire -- still leaves a profile. The harness picks the artifact path, giving a retried run a _retry suffix (and a numeric counter on collision) so it doesn't overwrite an earlier run's profile, and reports the written file via a structured test_status so it becomes a FAIL-named TestStatus marker the dashboards read and is downgraded to expected on a retried run. Differential Revision: https://phabricator.services.mozilla.com/D308521
1 parent 93441d7 commit b9b3c5a

2 files changed

Lines changed: 74 additions & 22 deletions

File tree

testing/xpcshell/head.js

Lines changed: 17 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -632,26 +632,21 @@ function _execute_test() {
632632
PerTestCoverageUtils.beforeTestSync();
633633
}
634634

635-
let timer;
636-
if (
637-
_Services.profiler.IsActive() &&
638-
!_Services.env.exists("MOZ_PROFILER_SHUTDOWN") &&
639-
_Services.env.exists("MOZ_UPLOAD_DIR") &&
640-
_Services.env.exists("MOZ_TEST_TIMEOUT_INTERVAL")
641-
) {
642-
timer = _Cc["@mozilla.org/timer;1"].createInstance(Ci.nsITimer);
643-
timer.initWithCallback(
644-
function upload_test_timeout_profile() {
645-
ChromeUtils.addProfilerMarker(
646-
"xpcshell-test",
647-
{ category: "Test", startTime },
648-
_TEST_NAME
649-
);
650-
_do_upload_profile();
651-
},
652-
parseInt(_Services.env.get("MOZ_TEST_TIMEOUT_INTERVAL")) * 1000 * 0.9, // Keep 10% of the time to gather the profile.
653-
timer.TYPE_ONE_SHOT
654-
);
635+
// If we don't finish before the harness's timeout, the profiler writes a
636+
// profile from its sampler thread, so a test that wedges its main thread
637+
// (e.g. a long synchronous run that never returns to the event loop) still
638+
// leaves a profile. Unlike a main-thread nsITimer this fires regardless of
639+
// what the main thread is doing. The harness picks the output path (in
640+
// MOZ_TEST_TIMEOUT_PROFILE_PATH) and reports the written file when it times
641+
// the test out.
642+
let scheduledProfileDump = false;
643+
let timeoutProfilePath = _Services.env.get("MOZ_TEST_TIMEOUT_PROFILE_PATH");
644+
if (timeoutProfilePath && _Services.profiler.IsActive()) {
645+
// Keep 10% of the time to write the profile before the harness kills us.
646+
let delaySeconds =
647+
parseInt(_Services.env.get("MOZ_TEST_TIMEOUT_INTERVAL")) * 0.9;
648+
_Services.profiler.scheduleDumpToFile(delaySeconds, timeoutProfilePath);
649+
scheduledProfileDump = true;
655650
}
656651

657652
try {
@@ -798,8 +793,8 @@ function _execute_test() {
798793
_PromiseTestUtils.uninit();
799794
}
800795

801-
if (timer) {
802-
timer.cancel();
796+
if (scheduledProfileDump) {
797+
_Services.profiler.cancelScheduledDump();
803798
}
804799

805800
// If MOZ_PROFILER_SHUTDOWN is set, the profiler got started from --profiler

testing/xpcshell/runxpcshelltests.py

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,9 @@ def __init__(
187187
# Retry in CI, but report results without retry when run locally to
188188
# avoid confusion and ease local debugging.
189189
self.retry = os.environ.get("MOZ_AUTOMATION") is not None
190+
# True only for the re-run of a test that failed the first time, so it
191+
# can name artifacts differently from the initial run.
192+
self.is_retry = kwargs.get("is_retry", False)
190193
self.verbose = verbose
191194
self.usingTSan = usingTSan
192195
self.usingCrashReporter = usingCrashReporter
@@ -454,6 +457,29 @@ def testTimeout(self, proc):
454457
if self.timeout_factor > 1:
455458
extra = {"timeoutfactor": self.timeout_factor}
456459

460+
# If the profiler dumped a profile from its sampler thread before we
461+
# killed the wedged process (scheduled by head.js via scheduleDumpToFile
462+
# since the main thread can't write one once it stops returning to the
463+
# event loop), report it here as a structured test_status, logged while
464+
# the test is still in progress so the artifact is linked to this test.
465+
# A structured message (unlike a raw log line) is downgraded to expected
466+
# on a retried run, and is recorded as a FAIL marker in the
467+
# resource-usage profile the dashboards read.
468+
profile_name = self.timeout_profile_name
469+
upload_dir = self.env.get("MOZ_UPLOAD_DIR")
470+
if (
471+
profile_name
472+
and upload_dir
473+
and os.path.isfile(os.path.join(upload_dir, profile_name))
474+
):
475+
self.log.test_status(
476+
self.test_object["id"],
477+
"",
478+
"FAIL",
479+
expected="FAIL" if (self.retry or self.timeoutAsPass) else expected,
480+
message=f"Test timed out; profile uploaded in {profile_name}",
481+
)
482+
457483
if self.retry:
458484
self.log.test_end(
459485
self.test_object["id"],
@@ -990,11 +1016,41 @@ def run_test(self):
9901016

9911017
testTimeoutInterval = self.harness_timeout * self.timeout_factor
9921018

1019+
self.timeout_profile_name = None
9931020
if not self.interactive and not self.debuggerInfo and not self.jsDebuggerInfo:
9941021
self.timer = Timer(testTimeoutInterval, lambda: self.testTimeout(proc))
9951022
self.timer.start()
9961023
self.env["MOZ_TEST_TIMEOUT_INTERVAL"] = str(testTimeoutInterval)
9971024

1025+
# When the profiler runs by default, have it dump a profile from its
1026+
# sampler thread shortly before this timeout fires (armed by
1027+
# head.js), so a test wedged in a synchronous run (where the main
1028+
# thread can never write one) still leaves a profile. We pick the
1029+
# artifact name here so the retry of a test that timed out doesn't
1030+
# overwrite the initial run's profile: the retry gets a "_retry"
1031+
# suffix, and a numeric counter is only added on an actual name
1032+
# collision (the same test listed in two manifests). The suffixes go
1033+
# before the test extension so the name still ends in e.g.
1034+
# ".js.json" as Treeherder expects. testTimeout reports this name.
1035+
upload_dir = self.env.get("MOZ_UPLOAD_DIR")
1036+
if (
1037+
upload_dir
1038+
and self.env.get("MOZ_PROFILER_STARTUP")
1039+
and "MOZ_PROFILER_SHUTDOWN" not in self.env
1040+
):
1041+
root, ext = os.path.splitext(os.path.basename(name))
1042+
if self.is_retry:
1043+
root += "_retry"
1044+
filename = f"profile_{root}{ext}.json"
1045+
i = 2
1046+
while os.path.exists(os.path.join(upload_dir, filename)):
1047+
filename = f"profile_{root}-{i}{ext}.json"
1048+
i += 1
1049+
self.timeout_profile_name = filename
1050+
self.env["MOZ_TEST_TIMEOUT_PROFILE_PATH"] = os.path.join(
1051+
upload_dir, filename
1052+
)
1053+
9981054
proc = None
9991055
process_output = None
10001056

@@ -2568,6 +2624,7 @@ def runTestList(
25682624

25692625
try_again_kwargs = kwargs.copy()
25702626
try_again_kwargs["retry"] = False
2627+
try_again_kwargs["is_retry"] = True
25712628
for test_object in self.try_again_list:
25722629
test = testClass(
25732630
test_object,

0 commit comments

Comments
 (0)