David Teller [:Yoric] - still alive but not very active Reporter
	Description • 10 years ago

In some cases, we crash during shutdown (possibly through the Terminator). It would be very useful to know, as part of the crash report, which of the AsyncShutdown blockers are active at the moment of the crash.

	David Teller [:Yoric] - still alive but not very active Reporter
	Comment 1 • 10 years ago

Aklotz suggests that calling `annotateCrashReport` directly is not a good idea as this requires re-serializing every single crash annotation after every single call. Since we might need to update our annotations a few hundred times during shutdown, this might be too much.

Apparently, there is a mechanism to extend the crash reporter that would be more appropriate.

Ted, Aklotz, can you confirm?

	(not currently active) Ted Mielczarek
	Comment 2 • 10 years ago

We generally go with one-off APIs to handle perf-sensitive annotations, like SetGarbageCollecting or SetEventloopNestingLevel:
http://hg.mozilla.org/mozilla-central/annotate/50b689feab5f/toolkit/crashreporter/nsExceptionHandler.cpp#l1803

Note that you also have to add code to write out your custom annotation:
http://hg.mozilla.org/mozilla-central/annotate/50b689feab5f/toolkit/crashreporter/nsExceptionHandler.cpp#l750

	David Teller [:Yoric] - still alive but not very active Reporter
	Comment 3 • 10 years ago

Since the data is collected from JS, is it acceptable to expose a new such API through nsICrashReporter?

Also, should we do this for v1 or should we start by calling the usual `annotateCrashReports`, see if it causes regressions, and then set out to add a custom annotator?

	(not currently active) Ted Mielczarek
	Comment 4 • 10 years ago

You are certainly welcome to do whatever you think needs doing. I'm not opposed to either option.

	David Teller [:Yoric] - still alive but not very active Reporter
	Comment 5 • 10 years ago

Attaching a first version without any specific optimization.

	Nathan Froyd [:froydnj]
	Comment 6 • 10 years ago

Comment on attachment 8503138 [details] [diff] [review]
Maintain up-to-date information on active blockers

Review of attachment 8503138 [details] [diff] [review]:
-----------------------------------------------------------------

WFM, going to profile to see if you need to improve this any?

	David Teller [:Yoric] - still alive but not very active Reporter
	Comment 7 • 10 years ago

That's the idea. I'm not sure exactly how, though.

	(No longer employed by Mozilla) Aaron Klotz
	Comment 8 • 10 years ago

Take a look at CrashReporter::SetEventloopNestingLevel for an example.

http://dxr.mozilla.org/mozilla-central/source/toolkit/crashreporter/nsExceptionHandler.cpp?from=CrashReporter::SetEventloopNestingLevel#1791
http://dxr.mozilla.org/mozilla-central/source/toolkit/crashreporter/nsExceptionHandler.cpp#746
http://dxr.mozilla.org/mozilla-central/source/toolkit/crashreporter/nsExceptionHandler.cpp#863

	David Teller [:Yoric] - still alive but not very active Reporter
	Comment 9 • 10 years ago

So, what about the following strategy?
1. finally, find a way to land bug 1044020;
2. wait a little for that bug to produce telemetry;
3. land this bug, and see how this impacts telemetry.

	Nathan Froyd [:froydnj]
	Comment 10 • 10 years ago

(In reply to David Rajchenbach-Teller [:Yoric] (use "needinfo") from comment #9)
> So, what about the following strategy?
> 1. finally, find a way to land bug 1044020;
> 2. wait a little for that bug to produce telemetry;
> 3. land this bug, and see how this impacts telemetry.

Sure.

	David Teller [:Yoric] - still alive but not very active Reporter
	Comment 11 • 9 years ago

/r/4089 - Bug 1077795 - Maintain up-to-date information on active AsyncShutdown blockers;r=froydnj
/r/4091 - Bug 1110681 - Instrumenting nsSearchService;r=florian

Pull down these commits:

hg pull review -r 82a0acb6c3c52533f28cf8aa26138614cd7db743

	David Teller [:Yoric] - still alive but not very active Reporter
	Comment 15 • 9 years ago

Not working on it atm, feel free to steal.