Closed Bug 1277260 Opened 9 years ago Closed 7 years ago

Annotate CCGraphBuilder::DescribeRefCountedNode() crashes with ClassName()

Categories

(Core :: XPCOM, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla58
Tracking Status
firefox58 --- fixed

People

(Reporter: mccr8, Assigned: mccr8)

References

(Blocks 2 open bugs)

Details

Attachments

(2 files)

No description provided.
Blocks: 1277280
Assignee: nobody → continuation
Summary: Annotate CCGraphBuilder::DescribeRefCountedNode() crashes with aObjName → Annotate CCGraphBuilder::DescribeRefCountedNode() crashes with ClassName()
Comment on attachment 8909379 [details] Bug 1277260, part 1 - Make PtrInfo into a class and mark it final. https://reviewboard.mozilla.org/r/180904/#review186128
Attachment #8909379 - Flags: review?(bugs) → review+
Comment on attachment 8909380 [details] Bug 1277260, part 2 - Add and use method to annotate CC crashes with a class name. https://reviewboard.mozilla.org/r/180906/#review186134 ::: xpcom/base/nsCycleCollector.cpp:2399 (Diff revision 1) > NS_IMETHODIMP_(void) > CCGraphBuilder::DescribeRefCountedNode(nsrefcnt aRefCount, const char* aObjName) > { > - MOZ_RELEASE_ASSERT(aRefCount != 0, "CCed refcounted object has zero refcount"); > - MOZ_RELEASE_ASSERT(aRefCount != UINT32_MAX, "CCed refcounted object has overflowing refcount"); > + mCurrPi->AnnotatedReleaseAssert(aRefCount != 0, > + "CCed refcounted object has zero refcount"); > + mCurrPi->AnnotatedReleaseAssert(aRefCount != UINT32_MAX, Hmm, not quite sure UINT32_MAX check is the right one anymore on 64bit systems at least, but no need to change here.
Attachment #8909380 - Flags: review?(bugs) → review+
Blocks: 1400978
Comment on attachment 8909380 [details] Bug 1277260, part 2 - Add and use method to annotate CC crashes with a class name. Requesting data collection approval. This just annotates a crash report with the name of a C++ class, so I don't think there is anything particularly privacy sensitive.
Attachment #8909380 - Flags: feedback?(rweiss)
Comment on attachment 8909380 [details] Bug 1277260, part 2 - Add and use method to annotate CC crashes with a class name. Maybe another data steward can take a look.
Attachment #8909380 - Flags: feedback?(liuche)
:mccr8, can you answer the 8 questions in this form here? https://docs.google.com/document/d/1SSn5w8DfCSkHWJS8DNTd7ya82diWRxaDUFM5aL4UDDo/edit This looks like it will be a very fast review with answers to those 8 questions.
Flags: needinfo?(continuation)
1) What are the motivating questions you want to answer with this data? Which C++ classes are involved in these cycle collector crashes? 2) Why does Mozilla need answers to this question? Are there benefits for users? Do we need this information to address business requirements? This information may allow me to fix crashes that users are experiencing. 3) List all proposed measurements. High-level description: C++ classes involved in cycle collector crashes, to be included in crash reports. Measurement type: Technical data. 4) How long will this data be collected? Forever. However, this data is only collected if the user actually crashes in this way, so if I manage to somehow fix all of these crashes (though that's unlikely) we'd stop collecting data. 5) What populations do you need to these measurements for? All channels. 6) Please provide a general description of how you will analyze this data. I will look at crash reports, see which C++ classes we're hitting this assertion for, and then manually audit those classes to see if I can find any bugs that would cause this crash. 7) What alternative methods did you consider to answer this question? Why were they not sufficient? I looked at some of these crash reports, but there's not enough information to solve them. Alternative approaches would be waiting for a fuzzing test case, or a URL that hits this crash found by our web spidering, but I haven't seen anything like that. 8) Can current instrumentation answer this question? No. 9) Where do you intend to share the results of your analysis? I will post anything I find in bug 1277280.
Flags: needinfo?(continuation)
Attachment #8909380 - Flags: feedback?(liuche)
For the "supplementary details", I will be monitoring crash stats, which is where the data will be recorded.
Flags: needinfo?(rweiss)
r+ 1) Is there documentation that describes the schema for the ultimate data set available publicly, complete and accurate? Yes, standard Telemetry documentation. 2) Is there a control mechanism that allows the user to turn the data collection on and off? (Note, for data collection not needed for security purposes, Mozilla provides such a control mechanism) Yes, users can opt out of Telemetry in the normal fashion. 3) If the request is for permanent data collection, is there someone who will monitor the data over time? Permanent request, though I assume :mccr8 will handle it. 4) Using the category system of data types on the Mozilla wiki, what collection type of data do the requested measurements fall under? This is type 1. 5) Is the data collection default-on or default-off? Default on for crashes. 6) Does the instrumentation include the addition of any new identifiers (whether anonymous or otherwise; e.g., username, random IDs, etc. See the appendix for more details)? No. 7) Is the data collection covered by the existing Firefox privacy notice? If unsure: escalate to legal. Yes. 8) Does there need to be a check-in in the future to determine whether to renew the data? (Yes/No) No
Flags: needinfo?(rweiss)
Comment on attachment 8909380 [details] Bug 1277260, part 2 - Add and use method to annotate CC crashes with a class name. https://reviewboard.mozilla.org/r/180906/#review189436 Provided the data review in response to the NI.
Attachment #8909380 - Flags: review+
Pushed by amccreight@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/36de9e91851f part 1 - Make PtrInfo into a class and mark it final. r=smaug https://hg.mozilla.org/integration/autoland/rev/e331b951c347 part 2 - Add and use method to annotate CC crashes with a class name. r=rweiss+418169,smaug
Attachment #8909380 - Flags: feedback?(rweiss)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: