Steps to reproduce: 1. Go to https://dl.dropboxusercontent.com/u/95157096/85f61cf7/fe80383b.swf. 2. When view the notification, do not kill plugin. 3. go to about:crashes. 4. Repeat the previous steps several times. 5. When view the notification, click on "Stop it". 6. Submit a crash report. See https://dl.dropboxusercontent.com/u/95157096/85f61cf7/1kd0r3jozm.mp4 Actual results: Generate several reports during the plugin hang. Crash reports: Note that only one report is marked 'hang'. bp-20f8b0a0-3710-4926-88b4-be6962160703 bp-7465cad6-9e5e-4098-af9a-a06ad2160703 bp-53999e06-669a-47bc-8d9d-897402160703 bp-ee956a0e-8ac9-411e-b248-6dea82160703 bp-79b10316-8444-4e14-a969-553742160703
Regression range: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=f9d32d5fc19a8cd5e408f7ff05283ff0c837061c&tochange=e309c8e399ad2bff9fe84993c52792fd688a5f81 Regressed by: bug 1262852
Thanks, this is precisely the information I needed, confirming an patch coming soon.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Assignee: nobody → gsvelto
Status: NEW → ASSIGNED
Created attachment 8767771 [details] [diff] [review] [PATCH] Do not take multiple minidumps during a hang This ensures that we don't take multiple minidumps from the same hang. It does so by storing the full minidump ID in the dump hash table replacing the browser ID so that we won't take a new one the next time we hit the same hang. I've also done a couple of code hygiene adjustments: - I've factored out the code taking the browser minidump; I've done so because this is thread-safe code and I wanted to highlight it - Updating the minidump ID in the hash-table is similarly thread-safe so the two go hand in hand - The dump ID and the hang data were always set and cleared together but had two separate functions to set them (SetHangData() and SetDumpId()). I've now consolidated both methods into a single one to make the intent clearer. The try run is here: https://treeherder.mozilla.org/#/jobs?repo=try&revision=9966397db256 If this approach is not appropriate or if it doesn't work properly (the try run hasn't finished running yet) I'll back out the patch in bug 1262852 to buy us more time to fix this.
Created attachment 8767942 [details] [diff] [review] [PATCH v2] Do not take multiple minidumps during a hang The previous patch had an issue that prevented it from being built when the crash reporter was disabled; this is addressed in this patch the rest is unchanged. The try run is here: https://treeherder.mozilla.org/#/jobs?repo=try&revision=838a2f792945
I wonder if there's some way we could write a test for this? We have a few plugin crash and hang tests under dom/plugins.
feel free to land the fix though, lets not hold that up.
(In reply to Jim Mathies [:jimm] from comment #5) > I wonder if there's some way we could write a test for this? We have a few > plugin crash and hang tests under dom/plugins. It's definitely doable, by letting the process hang detector run for two full iterations and then checking how many dumps we leave behind. I'll file a follow up for it.
Thanks for the quick review Jim! I've re-triggered a few oranges in the try run and if everything turns green I'll land this quickly.
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/mozilla-inbound/rev/1fecbe3e55c9 Do not take multiple minidumps during a hang r=jimm
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox50: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
You need to log in before you can comment on or make changes to this bug.