Closed Bug 792118 Opened 8 years ago Closed 7 years ago

Make sure "PluginHang" reports are processed as hangs

Categories

(Socorro :: Backend, task)

task
Not set

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: benjamin, Assigned: lars)

References

Details

(Whiteboard: [qa+])

Attachments

(1 file)

New-style hang reports won't have a HangID but will have PluginHang=1 in their raw JSON. This processor change synthesizes a HangID so that queries and the webapp continue to function. I'm going to put up a PR which is probably correct but untested and I'll need help getting it tested and landed.
https://github.com/mozilla/socorro/pull/839
Assignee: nobody → lars
Needs to be in production before November 19.  Lars, please review the PR and land.
Target Milestone: --- → 26
Assignee: lars → sdeckelmann
OS: Linux → All
Hardware: x86_64 → All
Whiteboard: [qa-]
Assignee: sdeckelmann → lars
Whiteboard: [qa-]
Samples can be had:

Prerequisites:

* Windows (can be done on other platforms with slightly different results)
* Firefox Nightly/Aurora build
* Sysinternals Process Explorer <http://technet.microsoft.com/en-us/sysinternals/bb896653.aspx>
* Change the pref "toolkit.crashreporter.pluginHangSubmitURL" to point to your server
* Change the pref "dom.ipc.plugins.timeoutSecs" to something smaller like 10

Then, to force a plugin to hang:

* Launch Firefox
* browse to a site that uses Flash such as youtube
* In process explorer, find your plugin-container process and suspend it
* Wait 10 seconds and Firefox should say that the plugin has crashed
unfortunately, I do not any access to a windows machine.  can I get a usable result if I were to try these instructions under Linux (my only option)
Yes, although you'll only get two minidumps (plugin and browser) instead of the four that come with Windows results.

You can probably suspend a process by sending it SIGSTOP or attaching with GDB.
Commits pushed to master at https://github.com/mozilla/socorro

https://github.com/mozilla/socorro/commit/84f9772c63fa284b8daa5c03b202b89f06c309eb
Bug 792118 - Make sure that new-style plugin hang reports (which have PluginHang=1 but no HangID) are processed correctly as plugin hangs.

https://github.com/mozilla/socorro/commit/b339563e8814f92f8dd06bb0cff4b44f909311ba
Merge pull request #915 from twobraids/hang2012

fixes Bug 792118 - new style PluginHang / HangID hack
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Whiteboard: [qa-]
brandt, what does [qa-] mean here? I really think this should be verified, because if FF18 hits beta without this fixed we could lose important stats.
:bsmedberg -- sorry for the confusion. I didn't realize the impact. I'll bump this to [qa+].

We use [qa-] for features that don't need testing by a QA and can be covered by dev.
Whiteboard: [qa-] → [qa+]
The attached crash pair is a synthesized crash with the attributes to trigger the new code in the bug.  The crash has PluginHang=1, but no HangID.  The correctly working processor should create a processed crash with hang_type=-1 and a hang_id starting with "fake-" followed by the crash's crash_id/uuid.
I'd like to have this verified against Firefox Aurora by changing the pref and submitting an actual crash.
Depends on: 810241
This is busted; all crashes are showing up as hangs.  Rolling back 26 in bug 810298.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
I've found the flaw in the logic.  

        if int(jsonDocument.get("Hang", False)):
          hangType = 1
        elif jsonDocument.get("PluginHang", False):
          hangType = -1
        elif 'hangid' in newReportRecordAsDict:
          hangType = -1
        else:
          hangType = 0

the problem is in the line "elif 'hangid' in newReportRecordAsDict".   "hangid" is _always_ in the crash.  If the crash isn't a hang, the value is NULL.  Since that part of the if chain is True, the hangType is always set to -1.  This, in turn, induces the SignatureUtility to prepend the word "hang" in front of the signature.  

the line should read:

    elif newReportRecordAsDict["hangid"]:

that way it senses the NULL case and doesn't force hang_type to -1 for all crashes.  Interestingly, the patch for the configmanized processor doesn't contain this flaw.
Let's test the fix in staging as bsmedberg suggests.
Commits pushed to master at https://github.com/mozilla/socorro

https://github.com/mozilla/socorro/commit/f6951f93bcaa6c8513a0d0e25e42bbdd509ea740
Revert "Bug 792118 - Make sure that new-style plugin hang reports (which have PluginHang=1 but no HangID) are processed correctly as plugin hangs."

This reverts commit 84f9772c63fa284b8daa5c03b202b89f06c309eb.

https://github.com/mozilla/socorro/commit/84bcd00e50b6daf86ef1448af4068ccdcd7549dc
Merge pull request #929 from selenamarie/revert-bug792118-master

Revert "Bug 792118 - Make sure that new-style plugin hang reports (which...
Commits pushed to master at https://github.com/mozilla/socorro

https://github.com/mozilla/socorro/commit/fdefc36bf43e00250e234a43f919b437790fc896
Bug 792118 - support for new-style plugin hangs (2nd version)

https://github.com/mozilla/socorro/commit/0982953426e857eeebb1fb31e1bcf2f318e775ab
Merge pull request #930 from twobraids/hang2012retry

Bug 792118 - support for new-style plugin hangs (2nd version)
Should this be re-resolved?  Needs to be released ASAP
I need independent confirmation from QA before I'm comfortable with this going into production.
Thanks lars -- we'll (QA) do a positive (comment 3) and negative test.
Status: REOPENED → RESOLVED
Closed: 7 years ago7 years ago
Resolution: --- → FIXED
QA Verified on stage -- lars thanks for the hand-holding on helping me test this.
+ testcase passes (comment 3) - https://crash-stats.allizom.org/report/index/bp-011802e9-0867-4864-b033-e306f2121115
+ also the tests run for https://bugzilla.mozilla.org/show_bug.cgi?id=792082#c11 also aply here
Status: RESOLVED → VERIFIED
(In reply to Matt Brandt [:mbrandt] from comment #19)
> QA Verified on stage -- lars thanks for the hand-holding on helping me test
> this.
> + testcase passes (comment 3) -
> https://crash-stats.allizom.org/report/index/bp-011802e9-0867-4864-b033-
> e306f2121115


I can't seem to pull the raw crash from stage  stephend can you redo this one against prod when you have a chance?
I followed comment 3 and changed toolkit.crashreporter.pluginHangSubmitURL to point to https://crash-reports.mozilla.com/submit, used process explorer to suspend Pandora's plugin-container for Flash, using nightly build Mozilla/5.0 (Windows NT 6.1; WOW64; rv:19.0) Gecko/19.0 Firefox/19.0

then got https://crash-stats.mozilla.com/report/index/19c967db-990e-4554-963a-3e4552121116.

Verified in prod, yay!
Yay -- thanks guys for doing this! Congrats on the release!
You need to log in before you can comment on or make changes to this bug.