831303 - Fix the the stop where we start counting the shutdown time

Status: RESOLVED FIXED

(Toolkit :: Telemetry, defect)

Description

[Rafael Ávila de Espíndola (:espindola) (not reading bugmail)]

Attachment: patch

https://tbpl.mozilla.org/?tree=Try&rev=062eb1142bb5

Comment 1

[(no longer active)]

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.

Comment 2

[Rafael Ávila de Espíndola (:espindola) (not reading bugmail)]

Thanks. I will push once 830809 is pushed.

Comment 3

[Rafael Ávila de Espíndola (:espindola) (not reading bugmail)]

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.

Comment 4

[Rafael Ávila de Espíndola (:espindola) (not reading bugmail)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/e3497f146c7c

Comment 5

[Randell Jesup [:jesup] (needinfo me)]

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.

Comment 6

[Rafael Ávila de Espíndola (:espindola) (not reading bugmail)]

(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?

Comment 7

[Randell Jesup [:jesup] (needinfo me)]

Attachment: 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.

Comment 8

[Rafael Ávila de Espíndola (:espindola) (not reading bugmail)]

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.

Comment 9

[Rafael Ávila de Espíndola (:espindola) (not reading bugmail)]

ttps://hg.mozilla.org/integration/mozilla-inbound/rev/2bee7c8730dc

Comment 10

[Rafael Ávila de Espíndola (:espindola) (not reading bugmail)]

I reported 832330 to track enabling it again.

Comment 11

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

https://hg.mozilla.org/mozilla-central/rev/e3497f146c7c
https://hg.mozilla.org/mozilla-central/rev/2bee7c8730dc