Closed Bug 831303 Opened 8 years ago Closed 8 years ago

Fix the the stop where we start counting the shutdown time

Categories

(Toolkit :: Telemetry, defect)

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla21

People

(Reporter: espindola, Unassigned)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

Comment on attachment 702823 [details] [diff] [review]
patch

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

::: toolkit/components/startup/nsAppStartup.cpp
@@ +378,5 @@
>          }
>        }
>      }
>  
> +    SAMPLE_MARKER("Shutdown start");

Doesn't this do part of what Benoit's patch was doing?  r=me anyways.

::: toolkit/components/telemetry/Telemetry.cpp
@@ +1973,5 @@
>  
>  namespace mozilla {
>  void
>  RecordShutdownStartTimeStamp() {
> +  static bool recorded = false;

Nit: surround this in an #ifdef DEBUG please.
Attachment #702823 - Flags: review?(ehsan) → review+
Thanks. I will push once 830809 is pushed.
Depends on: 830809
I just noticed the try job did only builds :-(

A new one is at https://tbpl.mozilla.org/?tree=Try&rev=601746acae25

I will push once that is done.
An inbound tree I've been using hit this MOZ_ASSERT(!recorded) on shutdown...  I have it in GDB, let me know if you want to see anything.  I'll hang onto it until at least the morning.
(In reply to Randell Jesup [:jesup] from comment #5)
> An inbound tree I've been using hit this MOZ_ASSERT(!recorded) on
> shutdown...  I have it in GDB, let me know if you want to see anything. 
> I'll hang onto it until at least the morning.

What is the backtrace? Is it reproducible? Can you get the backtrace of when it is first set too?
Attached file backtrace
I've only noticed it this once; not sure how many runs with it (I tend to ^C the browser when my test fails or quit from GDB)

This was an hour-long run of a video call; I'll note that getUserMedia was still active enough that the camera light was on at this point - but I don't think cleanup had happened yet to turn it off.
I am leaving on vacations really soon. BenWa can you try to debug this? If not I guess the best is to just comment the assert until I return. There is something strange with this function being called twice, but it will do the right thing: keep the last timestamp.
ttps://hg.mozilla.org/integration/mozilla-inbound/rev/2bee7c8730dc
I reported 832330 to track enabling it again.
https://hg.mozilla.org/mozilla-central/rev/e3497f146c7c
https://hg.mozilla.org/mozilla-central/rev/2bee7c8730dc
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
Depends on: 851169
You need to log in before you can comment on or make changes to this bug.