1770722 - Filing bugs for some crashes doesn't add the stack trace in Bugzilla's comment field

Status: RESOLVED FIXED

(Socorro :: Webapp, defect, P2)

Description

[Gabriele Svelto [:gsvelto]]

I was filing bugs this morning and I noticed that for some crashes I don't get the stack trace in the comment section in bugzilla, as if the template wasn't being filled. Here's a few where I experienced this behavior:

• https://crash-stats.mozilla.org/report/index/cd5388df-f0bb-4267-8790-bfd1b0220523
• https://crash-stats.mozilla.org/report/index/44fc2a5e-2e21-432e-8956-974460220523
• https://crash-stats.mozilla.org/report/index/0eec330c-d147-450c-b5d3-75a510220523

STR is:

• Open the crash
• Go to the Bugzilla tab
• Pick a component to file a bug
• The newly opened template doesn't have the stack trace in the comment section

Comment 1

[Will Kahn-Greene [:willkg] ET needinfo? me]

The stackwalker is reporting crashing_thread as null. The code for generating the bug sees the null and then doesn't add the stack details.

We could treat null as a 0 for the crashing_thread. I think that's what it's doing in the Details tab.

Gabriele: Would treating null as a 0 do the right thing here for meaningful bugs? Also, do you think it might be a bug in the stackwalker that crashing_thread is null? I'm guessing not, but figured it's worth asking.

Comment 2

[Gabriele Svelto [:gsvelto]]

I've checked where the null comes from and it's straight from the minidump. It's a case where the thread that caused the exception disappeared before we could capture it - hence the null value. I'd say that under these conditions the next best option is to use the contents of thread 0. It's not correct but it should always work.

Comment 3

[Will Kahn-Greene [:willkg] ET needinfo? me]

Ok.

In the Socorro codebase, we've got a bunch of places where the code looks at json_dump.crash_info.crashing_thread and then normalizes the value and does something. There's a CrashingThreadInfoRule in the processor pipeline that creates a crashing_thread field that's either an int or None. I think we should change that to be the value of json_dump.crash_info.crashing_thread or 0. Then we make sure all the code in the processor and webapp that look at the crashing thread look at that normalized value.

I think that'll work and not create new problems since we're sort of doing that already (very poorly).

Grabbing this to work on soon.

Comment 4

[Will Kahn-Greene [:willkg] ET needinfo? me]

This is turning out to be more complicated than I thought.

• Signature generation relies on crashing_thread=None to distinguish between when there's no frame information and when the stackwalker didn't identify a crashing thread.
• The report view relies on crashing_thread=None to decide whether to show the stack for a specific thread or the stacks for all the threads. I think we can remove this behavior.
• Tons of report view and other webapp tests use poorly thought out processed crash test data for their tests and it's tangled and all over the place. So making this one change affects a whole bunch of tests that are hard to fix.
• Making this change also breaks an integration test that it shouldn't have affected at all and I'm completely flummoxed by wtf is going on.

I'm throwing out the plan and going with the smallest fix in the link generation code instead.

Comment 5

[Will Kahn-Greene [:willkg] ET needinfo? me]

Gabriele: In the case we're showing thread 0 because crashing_thread is None, should we mention that in the bug report? Maybe something like this?:

Crash report: http://localhost:8000/report/index/0eec330c-d147-450c-b5d3-75a510220523

Reason: EXCEPTION_BREAKPOINT

No crashing thread identified; using thread 0.

Top 10 frames of crashing thread:

0 windows.security.integrity.dll wil::details::`dynamic atexit destructor for 'g_threadFailureCallbacks'' 
1 None @0xfffffffffffffffd 
2 ucrtbase.dll __crt_seh_guarded_call<int>::operator<<lambda_7777bce6b2f8c936911f934f8298dc43>, <lambda_f03950bc5685219e0bcd2087efbe011e>&, <lambda_3883c3dff614d5e0c5f61bb1ac94921c> > 
3 ucrtbase.dll execute_onexit_table 
4 ucrtbase.dll int __crt_state_management::wrapped_invoke<int  
5 windows.security.integrity.dll dllmain_crt_process_detach 
6 windows.security.integrity.dll dllmain_dispatch 
7 ntdll.dll LdrpCallInitRoutine 
8 ntdll.dll LdrShutdownProcess 
9 ntdll.dll RtlExitUserProcess 

Comment 6

[Gabriele Svelto [:gsvelto]]

(In reply to Will Kahn-Greene [:willkg] ET needinfo? me from comment #5)

Gabriele: In the case we're showing thread 0 because crashing_thread is None, should we mention that in the bug report?

Yes, that'd be very nice!

Comment 7

[Will Kahn-Greene [:willkg] ET needinfo? me]

Attachment: pr 6100: bug 1770722: use thread 0 in bug report

Comment 8

[Will Kahn-Greene [:willkg] ET needinfo? me]

willkg merged PR #6100: "bug 1770722: use thread 0 in bug report" in 45b2049.

Comment 9

[Will Kahn-Greene [:willkg] ET needinfo? me]

I pushed this out in bug #1777242. Marking as FIXED.