Closed Bug 1469086 Opened 3 years ago Closed 3 years ago

Crash in android.app.RemoteServiceException: Context.startForegroundService() did not then call Service.startForeground() at android.app.ActivityThread$H.handleMessage(ActivityThread.java)

Categories

(Firefox for Android Graveyard :: General, defect)

Unspecified
Android
defect
Not set
critical

Tracking

(firefox-esr60 unaffected, firefox60 unaffected, firefox61 unaffected, firefox62+ fixed)

RESOLVED FIXED
Firefox 62
Tracking Status
firefox-esr60 --- unaffected
firefox60 --- unaffected
firefox61 --- unaffected
firefox62 + fixed

People

(Reporter: calixte, Unassigned)

References

(Blocks 1 open bug)

Details

(Keywords: crash, regression)

Crash Data

This bug was filed from the Socorro interface and is
report bp-4d9fba9e-6e5a-404c-a67a-5fb910180615.
=============================================================

android.app.RemoteServiceException: Context.startForegroundService() did not then call Service.startForeground()
	at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1768)
	at android.os.Handler.dispatchMessage(Handler.java:106)
	at android.os.Looper.loop(Looper.java:164)
	at android.app.ActivityThread.main(ActivityThread.java:6494)
	at java.lang.reflect.Method.invoke(Native Method)
	at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:440)
	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:807)

=============================================================

There are 14 crashes in nightly 62 starting with buildid 20180612100032. In analyzing the backtrace, the regression may have been introduced by patch [1] to fix bug 1465102.

[1] https://hg.mozilla.org/mozilla-central/rev/0581ff7ccfef
Flags: needinfo?(vlad.baicu)
This seems to be the same situation as the one I mentioned in bug 1384866, originating from google's tracker https://issuetracker.google.com/issues/76112072. I think we should revert the patch for now, ni'ing Jan and Mike for their opinions as well
Flags: needinfo?(vlad.baicu)
Flags: needinfo?(michael.l.comella)
Flags: needinfo?(jh+bugzilla)
The notification code is hard to follow: are we certain we've had the opportunity to call `startForeground` before we call `stopService`?

One red flag in particular is that `onNotificationClose`, which calls stopService, is `synchronized` implying this class is intended to be accessed from multiple threads. It could be that we call startForegroundService and another thread quickly calls onNotificationClose before the new service can start (which will get queued to run on the UI thread when the intent is launched).

It doesn't help that this code is underdocumented (why does it need to be thread-safe? What is the difference between a NotificationClient and a NotificationHelper?).

But otherwise, I'd have to dig into the code further to understand what it actually does to provide a better response.
Flags: needinfo?(michael.l.comella)
Maybe small downloads (especially on fast connections) manage to complete fast enough that we want to cancel the ongoing "Downloading..." notification again (which entails stopping the service) before the NotificationService had a chance to execute onStartCommand?
Flags: needinfo?(jh+bugzilla)
Tracking for 62 since this is a top crash in 62 nightly.
bug 1465102 was backed out
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 62
See Also: → 1477041
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.