Re: Crash signature change: signatures now include abort messages
We are going to revert this change for now to unblock our release process, since it is clear that some changes need to be made, and I will be out all of next week. Now is the time to make comments about how that feature should behave! :) https://bugzilla.mozilla.org/show_bug.cgi?id=803779 On Thu, Apr 7, 2016 at 3:31 PM, Robert Kaiser wrote: > Sorry, that "look at the signatures" link should have been > https://crash-stats.allizom.org/search/?product=Firefox&signature=%5EAbort&_facets=signature&_columns=date&_columns=signature&_columns=abort_message#facet-signature > > I propose that before we add it to the signature, we remove /^\[.+\] > ###!!! ABORT: / from the abort message, I'm putting this in the bug as > well, we should discuss the details of what we do in there, I guess. > > KaiRo > > > Robert Kaiser schrieb: > > For one thing, the signature facet is more helpful to look at signatures: >> Abort&_facets=signature&_columns=date&_columns=signature&_columns=abort_message#facet-signature >> ;-) >> >> Hmm, I'm somewhat nervous about the fact that there is no signature that >> appears twice in this stage dataset. I think the part in [] is a process ID >> or something like that, which probably was added later than we made this >> spec, and which throws off this generation. I think we'll need to adjust >> this algorithm. >> >> Benjamin, what do you think? >> >> KaiRo >> >> >> Adrian Gaudebert schrieb: >> >>> Thanks Benjamin! >>> >>> Here's a search that shows what those new signatures look like on stage: >>> >>> https://crash-stats.allizom.org/search/?product=Firefox&signature=%5EAbort&_facets=abort_message&_columns=date&_columns=signature&_columns=abort_message >>> >>> Please let me know if that seems good or not before we push this change >>> to >>> prod. :) >>> >>> Cheers, >>> Adrian >>> >>> On Thu, Apr 7, 2016 at 3:27 AM, Bobby Holley >>> wrote: >>> >>> On Wed, Apr 6, 2016 at 2:34 PM, Benjamin Smedberg >>> > wrote: No, it does not catch MOZ_RELEASE_ASSERT, because that doesn't call > NS_DebugBreak and that is what does the AbortMessage annotation[1]. > NS_RUNTIMEABORT is the recommended way to reliably crash if you're in > XPCOM-y code. > > That's unfortunate, because according to MXR (and anecdotally) almost everyone uses MOZ_CRASH. >>> >>> ___ >>> Stability mailing list >>> stabil...@mozilla.org >>> https://mail.mozilla.org/listinfo/stability >>> >> >> ___ >> Stability mailing list >> stabil...@mozilla.org >> https://mail.mozilla.org/listinfo/stability >> > > ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Crash signature change: signatures now include abort messages
Sorry, that "look at the signatures" link should have been https://crash-stats.allizom.org/search/?product=Firefox&signature=%5EAbort&_facets=signature&_columns=date&_columns=signature&_columns=abort_message#facet-signature I propose that before we add it to the signature, we remove /^\[.+\] ###!!! ABORT: / from the abort message, I'm putting this in the bug as well, we should discuss the details of what we do in there, I guess. KaiRo Robert Kaiser schrieb: For one thing, the signature facet is more helpful to look at signatures: Abort&_facets=signature&_columns=date&_columns=signature&_columns=abort_message#facet-signature ;-) Hmm, I'm somewhat nervous about the fact that there is no signature that appears twice in this stage dataset. I think the part in [] is a process ID or something like that, which probably was added later than we made this spec, and which throws off this generation. I think we'll need to adjust this algorithm. Benjamin, what do you think? KaiRo Adrian Gaudebert schrieb: Thanks Benjamin! Here's a search that shows what those new signatures look like on stage: https://crash-stats.allizom.org/search/?product=Firefox&signature=%5EAbort&_facets=abort_message&_columns=date&_columns=signature&_columns=abort_message Please let me know if that seems good or not before we push this change to prod. :) Cheers, Adrian On Thu, Apr 7, 2016 at 3:27 AM, Bobby Holley wrote: On Wed, Apr 6, 2016 at 2:34 PM, Benjamin Smedberg wrote: No, it does not catch MOZ_RELEASE_ASSERT, because that doesn't call NS_DebugBreak and that is what does the AbortMessage annotation[1]. NS_RUNTIMEABORT is the recommended way to reliably crash if you're in XPCOM-y code. That's unfortunate, because according to MXR (and anecdotally) almost everyone uses MOZ_CRASH. ___ Stability mailing list stabil...@mozilla.org https://mail.mozilla.org/listinfo/stability ___ Stability mailing list stabil...@mozilla.org https://mail.mozilla.org/listinfo/stability ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Crash signature change: signatures now include abort messages
For one thing, the signature facet is more helpful to look at signatures: Abort&_facets=signature&_columns=date&_columns=signature&_columns=abort_message#facet-signature ;-) Hmm, I'm somewhat nervous about the fact that there is no signature that appears twice in this stage dataset. I think the part in [] is a process ID or something like that, which probably was added later than we made this spec, and which throws off this generation. I think we'll need to adjust this algorithm. Benjamin, what do you think? KaiRo Adrian Gaudebert schrieb: Thanks Benjamin! Here's a search that shows what those new signatures look like on stage: https://crash-stats.allizom.org/search/?product=Firefox&signature=%5EAbort&_facets=abort_message&_columns=date&_columns=signature&_columns=abort_message Please let me know if that seems good or not before we push this change to prod. :) Cheers, Adrian On Thu, Apr 7, 2016 at 3:27 AM, Bobby Holley wrote: On Wed, Apr 6, 2016 at 2:34 PM, Benjamin Smedberg wrote: No, it does not catch MOZ_RELEASE_ASSERT, because that doesn't call NS_DebugBreak and that is what does the AbortMessage annotation[1]. NS_RUNTIMEABORT is the recommended way to reliably crash if you're in XPCOM-y code. That's unfortunate, because according to MXR (and anecdotally) almost everyone uses MOZ_CRASH. ___ Stability mailing list stabil...@mozilla.org https://mail.mozilla.org/listinfo/stability ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Crash signature change: signatures now include abort messages
Thanks Benjamin! Here's a search that shows what those new signatures look like on stage: https://crash-stats.allizom.org/search/?product=Firefox&signature=%5EAbort&_facets=abort_message&_columns=date&_columns=signature&_columns=abort_message Please let me know if that seems good or not before we push this change to prod. :) Cheers, Adrian On Thu, Apr 7, 2016 at 3:27 AM, Bobby Holley wrote: > On Wed, Apr 6, 2016 at 2:34 PM, Benjamin Smedberg > wrote: > >> No, it does not catch MOZ_RELEASE_ASSERT, because that doesn't call >> NS_DebugBreak and that is what does the AbortMessage annotation[1]. >> NS_RUNTIMEABORT is the recommended way to reliably crash if you're in >> XPCOM-y code. >> > > That's unfortunate, because according to MXR (and anecdotally) almost > everyone uses MOZ_CRASH. > > ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Crash signature change: signatures now include abort messages
On Wed, Apr 6, 2016 at 2:34 PM, Benjamin Smedberg wrote: > No, it does not catch MOZ_RELEASE_ASSERT, because that doesn't call > NS_DebugBreak and that is what does the AbortMessage annotation[1]. > NS_RUNTIMEABORT is the recommended way to reliably crash if you're in > XPCOM-y code. > That's unfortunate, because according to MXR (and anecdotally) almost everyone uses MOZ_CRASH. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Crash signature change: signatures now include abort messages
No, it does not catch MOZ_RELEASE_ASSERT, because that doesn't call NS_DebugBreak and that is what does the AbortMessage annotation[1]. NS_RUNTIMEABORT is the recommended way to reliably crash if you're in XPCOM-y code. Bug 1104682 is filed for this, though. I'd love for somebody to take this. --BDS [1] http://mxr.mozilla.org/mozilla-central/source/xpcom/base/nsDebugImpl.cpp#392 On Wed, Apr 6, 2016 at 5:29 PM, Kyle Huey wrote: > Does this catch MOZ_RELEASE_ASSERT and similar? It would be nice to > distinguish those somehow in crash-stats. > > - Kyle > > On Wed, Apr 6, 2016 at 2:18 PM, Benjamin Smedberg > wrote: > >> A change just landed which will change the way crash signatures are >> computed for intentional aborts. Previously the crash signature would be >> "NS_DebugAbort | ". Now the signature will be "Abort | > message up to 80 chars> | ". >> >> This should make it much easier to distinguish various classes of abort. >> >> Thanks to Adrian Gaudebert for taking this in bug 803779! >> >> --BDS >> >> -- >> Benjamin Smedberg >> Engineering Manager, Firefox >> ___ >> dev-platform mailing list >> dev-platform@lists.mozilla.org >> https://lists.mozilla.org/listinfo/dev-platform >> > > -- Benjamin Smedberg Engineering Manager, Firefox ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Crash signature change: signatures now include abort messages
Does this catch MOZ_RELEASE_ASSERT and similar? It would be nice to distinguish those somehow in crash-stats. - Kyle On Wed, Apr 6, 2016 at 2:18 PM, Benjamin Smedberg wrote: > A change just landed which will change the way crash signatures are > computed for intentional aborts. Previously the crash signature would be > "NS_DebugAbort | ". Now the signature will be "Abort | message up to 80 chars> | ". > > This should make it much easier to distinguish various classes of abort. > > Thanks to Adrian Gaudebert for taking this in bug 803779! > > --BDS > > -- > Benjamin Smedberg > Engineering Manager, Firefox > ___ > dev-platform mailing list > dev-platform@lists.mozilla.org > https://lists.mozilla.org/listinfo/dev-platform > ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Crash signature change: signatures now include abort messages
A change just landed which will change the way crash signatures are computed for intentional aborts. Previously the crash signature would be "NS_DebugAbort | ". Now the signature will be "Abort | | ". This should make it much easier to distinguish various classes of abort. Thanks to Adrian Gaudebert for taking this in bug 803779! --BDS -- Benjamin Smedberg Engineering Manager, Firefox ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
