1405192 - Crash in java.lang.IllegalStateException: Trying to start session but it is already started at org.mozilla.gecko.telemetry.measurements.SessionMeasurements.recordSessionStart(SessionMeasurements.java)

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect, P1)

Description

[Ting-Yu Chou [:ting] (away)]

This bug was filed from the Socorro interface and is 
report bp-e9a9e7f0-7e4c-4fe3-bd57-8179a0171002.
=============================================================
#4 top crash of Nightly 20171001100334 on Android. The signature has been there for a while, there are 12226 reports in the past 6 months.

Comment 1

[Ting-Yu Chou [:ting] (away)]

Any ideas?

Comment 2

[Michael Comella (:mcomella) [NI reported issues only; no longer employed by Mozilla]]

Nevin, could the core fennec team take a look at this? I have my hands full with AS right now. Feel free to NI me if you get stuck!

Comment 3

[Nevin Chen(Not active on Bugzilla)]

Please help prioritize this.

Comment 4

[Wesly Huang (EPM)]

#12 in 57.0.4 while still seen in 59.0a1 (#40 top crasher) today so worth checking, adding to 59.5 sprint planning.

Comment 5

[Nevin Chen(Not active on Bugzilla)]

The fix will be similar to bug 1317191. I'll update the patch later.

Comment 6

[Nevin Chen(Not active on Bugzilla)]

Attachment: Bug 1405192 - Return when onResume is already called and add Telemetry ping for future investigationr. ?maliu

The fix will be similar to bug 1317191.

Review commit: https://reviewboard.mozilla.org/r/212004/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/212004/

Comment 7

[Nicholas Nethercote [inactive]]

This is the #1 Fennec topcrash in Nightly 20180202103243.

Comment 8

[Michael Comella (:mcomella) [NI reported issues only; no longer employed by Mozilla]]

[triage] Top 25 top crasher - will revisit from top crashers list.

Comment 9

[Marcia Knous [:marcia]]

Updating status flags - this crash is #4 top browser crash on 61.

One interesting comment: "It breaks when I'm in webmail google on android and request permission to record audio and record video (access to microphone and camera) and I deny.... "

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Nevin, can you please take another look at this?

Comment 11

[:sdaswani]

Vlad - this is top crasher that QA would like fixed sooner rather than later - can one of you three please pick it up ASAP.

Comment 12

[Vlad Baicu]

Attachment: Bug 1405192 - Return if onResume has already been called and onPause hasn't. Add telemetry ping for future investigations.

After investigating and analyzing the crash reports this type of scenario can occur due to multi-window or some popups. We are tracking this scenario for further investiagion with a telemetry event. Also added a constant for the hardcoded telemetry event when onPause gets called before onResume.

Review commit: https://reviewboard.mozilla.org/r/239676/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/239676/

Comment 13

[Vlad Baicu]

I see that Nevin already had submitted a patch for this issue. I have been unable to reproduce but it appears that these crashes occur whenever onResume is triggered twice without an onPause call in between. I have remade the patch with a slight addition and it should resolve the issue.

Comment 14

[Michael Comella (:mcomella) [NI reported issues only; no longer employed by Mozilla]]

Comment on attachment 8970919 [details]
Bug 1405192 - Return if onResume has already been called and onPause hasn't. Add telemetry ping for future investigations.

https://reviewboard.mozilla.org/r/239676/#review245970

> After investigating and analyzing the crash reports this type of scenario can occur due to multi-window or some popups.

This is good info! I'd actually recommend putting this directly into the comments since it's less likely to be discovered in extended commit summaries!

I don't think it's worth the turn-around time to add it but if you wanted to file a follow-up to add it, I could give it a quick r+.

---

Thinking about this problem, we have an assumption in the session measurements that onResume -> onPause will always be called in order. That assumption is clearly wrong so I wonder if instead of band-aiding it in TelemetryDelegate, we should just remove that assumption. That being said, this seems like it'd work to fix the crash and will give us reliable data for the (presumed) majority who don't have this misorder issue so it wfm.

Comment 15

[Michael Comella (:mcomella) [NI reported issues only; no longer employed by Mozilla]]

fwiw, we do have telemetry docs for UI telemetry but I don't think anyone updates them: https://firefox-source-docs.mozilla.org/mobile/android/fennec/uitelemetry.html

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a64ffd8dd132
Return if onResume has already been called and onPause hasn't. Add telemetry ping for future investigations. r=mcomella

Comment 17

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/a64ffd8dd132

Comment 18

[Ryan VanderMeulen [:RyanVM]]

No crashes on 61 since this landed, so marking that as verified. Vlad or Susheel, can you please nominate this patch for Beta approval so we can get this into 60 still? Thanks!

Comment 19

[:sdaswani]

Comment on attachment 8970919 [details]
Bug 1405192 - Return if onResume has already been called and onPause hasn't. Add telemetry ping for future investigations.

Approval Request Comment
[Feature/Bug causing the regression]:
[User impact if declined]:
[Is this code covered by automated tests?]:
[Has the fix been verified in Nightly?]:
[Needs manual test from QE? If yes, steps to reproduce]: 
[List of other uplifts needed for the feature/fix]:
[Is the change risky?]:
[Why is the change risky/not risky?]:
[String changes made/needed]:

Comment 20

[:sdaswani]

Comment on attachment 8970919 [details]
Bug 1405192 - Return if onResume has already been called and onPause hasn't. Add telemetry ping for future investigations.

I'll let Vlad do it so he can fill out the questions appropriately.

Comment 21

[Julien Cristau [:jcristau]]

Comment on attachment 8970919 [details]
Bug 1405192 - Return if onResume has already been called and onPause hasn't. Add telemetry ping for future investigations.

In the interest of time I'll approve for 60.0b17 and 60.0rc now.  We should still have the form filled out for the record though, so leaving the ni on Vlad.

Comment 22

[:sdaswani]

(In reply to Julien Cristau [:jcristau] from comment #21)
> Comment on attachment 8970919 [details]
> Bug 1405192 - Return if onResume has already been called and onPause hasn't.
> Add telemetry ping for future investigations.
> 
> In the interest of time I'll approve for 60.0b17 and 60.0rc now.  We should
> still have the form filled out for the record though, so leaving the ni on
> Vlad.

Thanks I forgot he is on PTO but he will fill the form when he is back.

Comment 23

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/releases/mozilla-release/rev/8e5321eb2d23
https://hg.mozilla.org/releases/mozilla-beta/rev/873f0487a7352e24b3c839fb31f71a63310c3174 (FIREFOX_60b_RELBRANCH)

Comment 24

[Vlad Baicu]

Approval Request Comment
[Feature/Bug causing the regression]: onResume called more than once with no onPause call in between. Examples of such occurences include usage of multi-window or some popups.
[User impact if declined]: The bug will continue to be a top crasher.
[Is this code covered by automated tests?]: Not that I am aware of.
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]:  No
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: No
[Why is the change risky/not risky?]: We are simply adding a check in case this event happens.
[String changes made/needed]: None