Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[audiofileplayer] The play stops or doesn't start and there are crashes and ANRs #120

Open
mr-mmmmore opened this issue Sep 2, 2021 · 21 comments

Comments

@mr-mmmmore
Copy link

Hi,

Since our app has been on production a few weeks ago I've been seing a rather high amount of crashes and ANRs related to this plugin on Google Play. I was waiting to have more information from user feedbacks to create an issue, so here it is.

Today a user complained about the audio play not starting, or stopping randomly while using our app. I have been able to link the following crash and ANR to this specific user:

Crash

java.lang.NullPointerException:
at com.google.flutter.plugins.audiofileplayer.AudiofileplayerPlugin.onMediaButtonClick (AudiofileplayerPlugin.java:25)
at com.google.flutter.plugins.audiofileplayer.AudiofileplayerService$MediaSessionCallback.onMediaButtonEvent (AudiofileplayerService.java:92)
at android.support.v4.media.session.MediaSessionCompat$Callback$StubApi21.onMediaButtonEvent (MediaSessionCompat.java:2)
at android.support.v4.media.session.MediaSessionCompatApi21$CallbackProxy.onMediaButtonEvent (MediaSessionCompatApi21.java:2)
at android.media.session.MediaSession$CallbackMessageHandler.handleMessage (MediaSession.java:1578)
at android.os.Handler.dispatchMessage (Handler.java:107)
at android.os.Looper.loop (Looper.java:237)
at android.app.ActivityThread.main (ActivityThread.java:8167)
at java.lang.reflect.Method.invoke (Native Method)
at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run (RuntimeInit.java:496)
at com.android.internal.os.ZygoteInit.main (ZygoteInit.java:1100)

ANR
Context.startForegroundService() did not then call Service.startForeground(): ServiceRecord{4a438d1 u0 myapp.bundleid.com/com.google.flutter.plugins.audiofileplayer.AudiofileplayerService}

This user is on Android 10 (SDK 29).

The app uses the latest plugin (v. 2.0.1) with Flutter 2.2.3.

@monkeyswarm
Copy link
Collaborator

Hi, do you have more of this crash log? I'm wondering if there is a line that says "Unsupported eventCode:"

@monkeyswarm
Copy link
Collaborator

As for the ANR, I don't see why it would take 5 seconds to go from AudiofileplayerService.java line 318 to 328

Some SO answers suggest putting 'startForeground' within the Service's methods, so we could try moving it from onPlay into the service onCreate or onStartCommand.

Can you also try running your app to start the service while the screen is locked? Does that cause the ANR (since the notification is not visible?)

@mr-mmmmore
Copy link
Author

Hi, do you have more of this crash log? I'm wondering if there is a line that says "Unsupported eventCode:"

No this is all I have for this crash. But there is another crash which occurs 4 times more often than his one. I don't have user feedback about it but yet it may help:

java.lang.IllegalStateException: 
  at io.flutter.embedding.engine.dart.DartMessenger$Reply.reply (DartMessenger.java:35)
  at io.flutter.plugin.common.MethodChannel$IncomingMethodCallHandler$1.error (MethodChannel.java:14)
  at com.google.flutter.plugins.audiofileplayer.AudiofileplayerPlugin.lambda$onLoad$1 (AudiofileplayerPlugin.java:34)
  at com.google.flutter.plugins.audiofileplayer.AudiofileplayerPlugin.lambda$onLoad$1$AudiofileplayerPlugin (AudiofileplayerPlugin.java)
  at com.google.flutter.plugins.audiofileplayer.-$$Lambda$AudiofileplayerPlugin$Gbvdvgt56UgLn60XCEjyHDNyAwg.onRemoteLoadComplete (lambda:11)
  at com.google.flutter.plugins.audiofileplayer.RemoteManagedMediaPlayer.onError (RemoteManagedMediaPlayer.java:3)
  at android.media.MediaPlayer$EventHandler.handleMessage (MediaPlayer.java:3338)
  at android.os.Handler.dispatchMessage (Handler.java:106)
  at android.os.Looper.loop (Looper.java:164)
  at android.app.ActivityThread.main (ActivityThread.java:6556)
  at java.lang.reflect.Method.invoke (Native Method)
  at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run (RuntimeInit.java:438)
  at com.android.internal.os.ZygoteInit.main (ZygoteInit.java:807)

(or I can create a dedicated issue for this crash if you prefer).

@mr-mmmmore
Copy link
Author

As for the ANR, I don't see why it would take 5 seconds to go from AudiofileplayerService.java line 318 to 328

Some SO answers suggest putting 'startForeground' within the Service's methods, so we could try moving it from onPlay into the service onCreate or onStartCommand.

I assume you mean the answers to this SO question, right? So it seems that the problem comes from a design flow in Android where the service must be started within 5 seconds but the system doesn't garantee that the service will be actually started nor does it check that an attempt to start it was made within this delay.

Several answers say that the startService call must be made in onCreate or in onStartCommand, or in both (this guy says that moving the call from onStartCommand to onCreate did the trick for him). This answer summarizes 3 things that must be carefully handled:

  1. Calling startService from onCreate or onStartCommand (but several answers say it should be in onCreate).
  2. Set the NOTIFICATION_ID to something else than 0 (but the comments to this SO answer also say it should be a small number).
  3. stopSelf must not be called before startForeground.

Regarding the 3. it seems that sometimes stopSelf is called before the service has had a chance to start, which causes errors. May be adding some checks before calling it could be added? Or before starting the service as this answer suggests? This answer goes further by saying that it is not needed for Android > 7.

But some other answer say that since startForegroundService has a design flaw the ANRs and crashes can't be avoided totally and that other approachs are to be preferred, such as using JobScheduler (which seems to imply a lot of refactoring), or using Context.bindService.

Regarding the use of bindService, This answer proposes a solution that seems to completely work in an app with 5M of users.

So what do you think? Using bindService seems worth a try, but it may imply more work, I can't evaluate this as I am not an Android developer. On the other hand the 3 numbered fixes listed above seem to be more direct solutions but they don't guarantee to completely get rid of the problem.

Can you also try running your app to start the service while the screen is locked? Does that cause the ANR (since the notification is not visible?)

I can start the app while the screen is locked but not trigger the play as navigating through several screens is needed to do it.

@monkeyswarm
Copy link
Collaborator

Yes, open a new issue for the post two above this one, that seems like a flutter embedding issue.

@monkeyswarm
Copy link
Collaborator

monkeyswarm commented Sep 6, 2021

I tried calling stopSelf() immediately (synchronously) after startForegroundService(), and there was not an issue (the service started successfully, though, which is strange). It's possible that calling it asynchronously would have other behavior.

I'll try moving startForeground() into the service onCreate

@monkeyswarm
Copy link
Collaborator

monkeyswarm commented Sep 7, 2021

I've tried moving startForeground() into the service.onCreate(). I did not get ANRs before, but now I do, when, in the example app, I

  1. Press button in card 4 (to start background audio and start the service)
  2. open the notification, pull it down to expand it, and hit the 'stop' (not the 'pause') button
  3. back in the app, press buttin in card 4 again to restart audio (and start the service again)

Always hits an ANR.

@mr-mmmmore
Copy link
Author

Yes, open a new issue for the post two above this one, that seems like a flutter embedding issue.

Actually this IllegalStateException problem seems to be a duplicate of #54.

@mr-mmmmore
Copy link
Author

mr-mmmmore commented Sep 7, 2021

I tried calling stopSelf() immediately (synchronously) after startForegroundService(), and there was not an issue (the service started successfully, though, which is strange). It's possible that calling it asynchronously would have other behavior.

Isn't it because stopSelf() should be called from inside the service and the service isn't started yet when you call startForegroundService() ? May be it would do what you want by using Context.stopService(intent)?

@mr-mmmmore
Copy link
Author

I've tried moving startForeground() into the service.onCreate(). I did not get ANRs before, but now I do, when, in the example app, I

1. Press button in card 4 (to start background audio and start the service)

2. open the notification, pull it down to expand it, and hit the 'stop' (not the 'pause') button

3. back in the app, press buttin in card 4 again to restart audio (and start the service again)

Always hits an ANR.

  1. Will cause the service to stop, and onCreate() may not have been not called later in 3.? Maybe adding the startForeground() call in onStartCommand() too could help?

Also I've found that there is a safe variant of stopSelf, stopSelfResult (doc), which takes a startId parameter that allows to prevents stopping the service if a new start command has been issued meanwhile. The service will stop only if the latest startId is passed. It may help in preventing errors where fast interactions lead to attempts to use the service while it had been stopped shortly before.

@monkeyswarm
Copy link
Collaborator

I did some more digging, and it looks like startForeground must be called in onStartCommand, not onCreate. I've tried that change, I a don't get the ANR I previously mentioned.

I mention above (#120 (comment)) trying to force a crash or ANR by calling stopSelf right after starting the service, but I don't hit any issues. The service is started (and the notification displayed), so I think that call to stopSelf just has no effect if the service hasn't completed starting up yet. But I could certainly be wrong.

I'm ok moving the startForeground into the Service.onStartCommand to see if it fixes this for you. Do you use this library via pub.dev, or do you have the code locally available (i.e. can you edit the Android code yourself to run your app with a suggested change)?

@arodriguezgb
Copy link
Contributor

Hey man, as you recall, i been suffering from this on my production app aswell, so im willing to try these changes if you dont want them to be live yet, you can do a new branch with these fixes and ill point my pubspec there

@mr-mmmmore
Copy link
Author

mr-mmmmore commented Sep 8, 2021

Same for me, I can use a modified version, either on a branch or with code pasted here. I will start by running some tests on the few devices I have and then publish the app if it works well.

@monkeyswarm
Copy link
Collaborator

Ok, I've commited that change onto this branch (note it is on my fork, not on the google upstream):

https://github.com/monkeyswarm/flutter.plugins/pull/new/startForegroundChange

Let me know if you are able to incorporate that and test. If things seem to work, I'll push it to upstream/master and publish a new version on pub.dev. Let me know, thanks.

@mr-mmmmore
Copy link
Author

OK thanks! I'll test it in the coming days when I'm done with my current feature and if it works I'll include it in the next release of my app.

@arodriguezgb
Copy link
Contributor

Let me know if it works mr more , I will roll out to my beta branch and see how it goes because sadly none of my testing devices can reproduce this issue, however i have a lot of vitals with these.

@arodriguezgb
Copy link
Contributor

Also the DART REPLY crash is happening for me on many devices on the vitals, like the other one, i have not been able to reproduce on my end to be more helpful.

@mr-mmmmore
Copy link
Author

So far I haven't seen problems with the fork, but I need to test on 2 more devices, which I'll do tomorrow. If it's still good I'll submit my app next week and we'll start to see if the crashes and/or ANR have decreased in two weeks I think. I'll keep you updated.

@monkeyswarm
Copy link
Collaborator

monkeyswarm commented Sep 17, 2021 via email

@mr-mmmmore
Copy link
Author

Based on my own limited tests on 3 Android devices your changes don't seem to cause new problems. So I'll push an update of my app today on the stores. We'll see in the coming weeks if the crashes/ANR decrease.

But as you say if they don't get worse the change is worth being merged anyway as starting the service in onStartCommand seems to be the right way to do it.

@mr-mmmmore
Copy link
Author

So far so good, my new app version was released 8 days ago and since then most of the crashes and ANR have disappeared! So it seems that your changes have worked @monkeyswarm! Thank you!

There are two crashes that remain though: the Reply already submitted one, already referenced in #54 and a NullPointerException in AudiofileplayerPlugin.onMediaButtonClick for which I'll create an issue when the occurences grow (it has happened only once).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants