Missing error handling for reading test list in reftest.jsm can cause infinite hangs

RESOLVED FIXED in Firefox 60

Status

enhancement
RESOLVED FIXED
Last year
Last year

People

(Reporter: whimboo, Assigned: whimboo)

Tracking

unspecified
mozilla61
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox60 fixed, firefox61 fixed)

Details

Attachments

(1 attachment)

I noticed a permanent hang of the reftest harness when running the tests by using a profile with a Unicode character in its path. Further investigation has been shown that the problem here is mainly because there is no error handling code for the following call to `OS.File.read()`:

https://dxr.mozilla.org/mozilla-central/rev/83de58ddda2057f1cb949537f6b111e3b115ea3d/layout/tools/reftest/reftest.jsm#367-371

Whenever that fails `StartTests()` is not being called, and the extension performs no single test which then causes the browser to not output anything. As result the job times out after 370s without receiving any output. See bug 1441580.

What we have to do here is to handle a possible exception, and early exit.
The exact failure is something like:

> REFTEST ERROR | ***** failed to load test: Unix error 2 during operation open on file /var/folders/4k/sf4gz5fn3kl9hr3nd7pzbvhc0000gn/T/tmpP_Ga7H.mozrunner🍪/reftests.json (No such file or directory)

I feel there is a problem with `OS.File` which is kinda broken when Unicode characters are in the path. But that should actually be a different bug.

A patch to prevent the hang should be up soon.
Assignee: nobody → hskupin
Status: NEW → ASSIGNED
The problem is not with `Os.File` but with `prefs.getCharPref("reftest.tests", null)` which doesn't have Unicode support. To fix that the reftest harness has to use `prefs.getStringPref()`. I will file a separate bug for this change.
Comment on attachment 8966552 [details]
Bug 1452913 - Prevent hang of reftests when reading test objects list fails.

https://reviewboard.mozilla.org/r/235272/#review240998
Attachment #8966552 - Flags: review?(gbrown) → review+
Pushed by hskupin@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f079b11f50b7
Prevent hang of reftests when reading test objects list fails. r=gbrown
https://hg.mozilla.org/mozilla-central/rev/f079b11f50b7
Status: ASSIGNED → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
This patch makes reftests more stable. It would be good to see this test-only patch uplifted to beta.
Whiteboard: [checkin-needed-beta]
Whiteboard: [checkin-needed-beta]
You need to log in before you can comment on or make changes to this bug.