1594577 - Try to record BHR hangs which precede forced shutdowns

Status: RESOLVED FIXED

(Core :: XPCOM, enhancement)

Description

[Alex Thayer [:alexical] (she/her)]

Currently if a user forcibly terminates the main process because it's unresponsive, we won't collect the BHR hang. I think we can remedy this by writing hangs out to disk if we pass a certain threshold of, say, eight seconds (the current definition of a "permahang" in BHR terms). At that point the overhead of trying to persist the stack to disk should be a drop in the pond, and it could provide some very valuable data about egregious hangs that users experience.

Comment 1

[Alex Thayer [:alexical] (she/her)]

Attachment: Bug 1594577 - Record hangs which precede forced shutdowns r?froydnj

In short - if a user forcibly terminates the browser because it seems
to be permanently hung, we currently do not get a change to record the
hang. This is unfortunate, because these likely represent the most
egregious hangs in terms of user frustration. This patch seeks to
address that.

If a hang exceeds 8192ms (the current definition of a "permahang" in
existing BHR terms), then we decide to immediately persist it to disk,
in case we never get a chance to return to the main thread and
submit it. On the next start of the browser, we read the file from
disk on a background thread, and just submit it using the normal
mechanism.

Regarding the handling of the file itself, I tried to do the simplest
thing I could - as far as I can tell there is no standard simple
serialization mechanism available directly to C++ in Gecko, so I just
serialized it by hand. I didn't take any special care with endianness
or anything as I can't think of a situation in which we really care
at all about these files being transferable between architectures. I
directly used PR_Write / PR_Read instead of doing something fancy
like memory mapping the file, because I don't think performance is a
critical concern here and it offers a simple protection against
reading out of bounds.

Comment 2

[Alex Thayer [:alexical] (she/her)]

Attachment: Data Review Request

Comment 3

[Chris H-C :chutten]

Comment on attachment 9108837 [details]
Data Review Request

DATA COLLECTION REVIEW RESPONSE:

    Is there or will there be documentation that describes the schema for the ultimate data set available publicly, complete and accurate?

Yes. This collection is Telemetry and is documented in [its documentation](https://firefox-source-docs.mozilla.org/toolkit/components/telemetry/data/backgroundhangmonitor-ping.html).

    Is there a control mechanism that allows the user to turn the data collection on and off?

Yes. This collection is Telemetry so can be controlled through Firefox's Preferences.

    If the request is for permanent data collection, is there someone who will monitor the data over time?

Yes, Doug Thayer is responsible.

    Using the category system of data types on the Mozilla wiki, what collection type of data do the requested measurements fall under?

Category 1, Technical.

    Is the data collection request for default-on or default-off?

Default on for Nightly only.

    Does the instrumentation include the addition of any new identifiers?

No.

    Is the data collection covered by the existing Firefox privacy notice?

Yes.

    Does there need to be a check-in in the future to determine whether to renew the data?

No. This collection is permanent.

---
Result: datareview+

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/bd42216f7b63
Record hangs which precede forced shutdowns r=froydnj

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f7dae5e23628
Fix ESlint failure. r=me CLOSED TREE

Comment 6

[Cristian Brindusan [:cbrindusan]]

Backed out 2 changesets (Bug 1594577) for build bustages at HangDetails.cpp.

Backout: https://hg.mozilla.org/integration/autoland/rev/d7e41714afd172c1a027bca721a2e3bc319d0257

Push that started the failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=pending%2Crunning%2Csuccess%2Ctestfailed%2Cbusted%2Cexception&revision=bd42216f7b6309c683bcc8d9d63c26a834d08d04&selectedJob=276505424

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=276505424&repo=autoland&lineNumber=33868

Please note that there is also an ESlint failure:
https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&revision=bd42216f7b6309c683bcc8d9d63c26a834d08d04&selectedJob=276505391

Comment 7

[Cristian Brindusan [:cbrindusan]]

and some mochitest failures: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=276511743&repo=autoland&lineNumber=1160

Comment 8

[Alex Thayer [:alexical] (she/her)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=3f0f490be3c55dfab1d37215c55addbe427d5204

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c7b4e89f5ee6
Record hangs which precede forced shutdowns r=froydnj

Comment 10

[Narcis Beleuzu [:NarcisB]]

Backed out for xpcshell failures on test_watchdog_hibernate.js

Backout link: https://hg.mozilla.org/integration/autoland/rev/08dc799f5943e645d811b0c4ab37713a01ebe94b
Log link: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=277301048&repo=autoland&lineNumber=2757

Comment 11

[Narcis Beleuzu [:NarcisB]]

There were also GTest failures like https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=277300473&repo=autoland&lineNumber=1877

Comment 12

[Narcis Beleuzu [:NarcisB]]

Also dt failures on browser_dbg-toolbox-unselected-pause.js
Log link: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=277320157&repo=autoland&lineNumber=13834

Comment 13

[Alex Thayer [:alexical] (she/her)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=454dc10adbf2cce9fafe01d24cbe3fb9b76e3d18

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/eb8741186210
Record hangs which precede forced shutdowns r=froydnj

Comment 15

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/eb8741186210