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

void com.google.ads.interactivemedia.v3.api.AdsManager.resume() nullpointer exception #3879

Closed
vinayjoglekar opened this issue Feb 22, 2018 · 11 comments
Assignees
Labels

Comments

@vinayjoglekar
Copy link

Hi Team,
When pre roll ad is playing and app goes in background and comes back in foreground, app crashes. Here is the stack trace.

FATAL EXCEPTION: main Process: PID: 22373 java.lang.NullPointerException: Attempt to invoke interface method 'void com.google.ads.interactivemedia.v3.api.AdsManager.resume()' on a null object reference at com.google.android.exoplayer2.ext.ima.ImaAdsLoader.attachPlayer(ImaAdsLoader.java:396) at com.google.android.exoplayer2.source.ads.AdsMediaSource$2.run(AdsMediaSource.java:219) at android.os.Handler.handleCallback(Handler.java:789) at android.os.Handler.dispatchMessage(Handler.java:98) at android.os.Looper.loop(Looper.java:164) at android.app.ActivityThread.main(ActivityThread.java:6809) at java.lang.reflect.Method.invoke(Native Method) at com.android.internal.os.Zygote$MethodAndArgsCaller.run(Zygote.java:240) at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:767)

Here are my onresume and onpause methods of fragment:
@Override public void onResume() { super.onResume(); player.init(getActivity(), playerView); }

@Override public void onPause() { super.onPause(); player.reset(); player.release(); playerView.getPlayer().release(); }

Please help me with the same.

@andrewlewis
Copy link
Collaborator

Which ExoPlayer version are you using? I can't see a path where we fail to load ads (meaning that the ads manager is null) but IMA has paused content, which would trigger this error. I can't reproduce this issue in the demo app.

If you can reproduce this issue please could you set the DEBUG constant in ImaAdsLoader to true, reproduce the issue, then take a bug report with adb bugreport? Thanks.

@vinayjoglekar
Copy link
Author

I am using both exoplayer and IMAextension having version 2.6.1. I can''t really access ImaAdsLoader class as I am calling lib from gradle.

@andrewlewis
Copy link
Collaborator

When do you release the ImaAdsLoader? Is it possible that you're releasing it then preparing the player with an AdsMediaSource pointing to the released ads loader?

@ojw28
Copy link
Contributor

ojw28 commented Mar 5, 2018

Closing on the assumption the suggestion by @andrewlewis was the problem, due to lack of follow up. If that's not the issue, please reply here and we'll re-open the issue.

@ojw28 ojw28 closed this as completed Mar 5, 2018
@fpittersacc
Copy link

fpittersacc commented Apr 23, 2018

Hi @ojw28 , @andrewlewis

It seems this is happening due to a race condition when releasing a "paused" IMA and player/media source being prepared.

We are not able to reproduce it on our current tapplication or the IMA demo without forcing it, but NPEs are still being reported on our trackers after we have upgraded to Exo 2.7.x

One easy way to force it on the IMA demo is:

  1. Force release on second resume (will be triggered when returning from background):
 @Override
    public void onResume() {
        super.onResume();

        player.init(this, playerView);

        if (resumedSecondTime) {
            player.release();
            finish();
        }
        resumedSecondTime = true;
    }
  1. Launch IMA demo
  2. While playing the Ad, go to background:
  3. Return to IMA demo from recent Apps.

The race condition seems to be happening on AdsMediaSource.prepareSource, that queues a call to AdsLoader.attachPlayer into mainHandler. If the player is released before the runnable runs then we reach attachPlayer with a released ImaAdsLoader.

A possible fix could be clearing the mainHandler on AdsMediaSource.releaseSource.

Let me know if you need something else.
Thanks

@andrewlewis andrewlewis reopened this Apr 23, 2018
@andrewlewis
Copy link
Collaborator

@fpittersacc Thanks for the information! Is it possible that the application is releasing the player immediately after initializing (as you say), but then the app is also calling ImaAdsLoader.release as part of the same cycle of the main looper? The attach/detach runnables are queued and there will be a crash in ImaAdsLoader.attachPlayer.

I think this is the same as what you said but it's the fact that the ImaAdsLoader has been released that's significant (not the player). Perhaps we don't see this in the demo app because we release the ads loader in onDestroy which isn't called from any of the other lifecycle methods in the demo app.

If this makes sense, I can fix it by making the attach/detach methods no-ops if the loader is already released.

@andrewlewis
Copy link
Collaborator

@fpittersacc Does the suggestion above make sense? Is it possible that your app releases the ImaAdsLoader right after it releases the player? Thanks!

@ojw28
Copy link
Contributor

ojw28 commented Jun 5, 2018

Closing due to lack of updates.

@ojw28 ojw28 closed this as completed Jun 5, 2018
@fpittersacc
Copy link

@andrewlewis , sorry I've missed you questions.

Yes, we are releasing the player and the ImaAdsLoader at the same time on some situations, so it's the queued runnable running after ImaAdsLoader has been released.

We have included a released flag on ImaAdsLoader and we discard attach/detach callbacks if active. But maybe the right approach would be clearing queued runnables in the handler, not clear how to do it thought.

Let me know if I can help with anything.

@andrewlewis andrewlewis reopened this Jun 6, 2018
@brinkkemper
Copy link

I have a similar NPE appearing in our crashlogs. But it's on start() instead of resume.
Can't reproduce it either in our app.

java.lang.NullPointerException Attempt to invoke interface method 'void com.google.ads.interactivemedia.v3.api.AdsManager.start()' on a null object reference 
    ImaAdsLoader.java:589 com.google.android.exoplayer2.ext.ima.a.loadAd
    IMASDK:36 com.google.obf.hm.a
    IMASDK:151 com.google.obf.hj.e
    IMASDK:45 com.google.obf.hj.a
    IMASDK:39 com.google.obf.hk.b
    IMASDK:4 com.google.obf.hk$1.shouldOverrideUrlLoading
    WebViewClient.java:73 android.webkit.WebViewClient.shouldOverrideUrlLoading
    PG:89 yk.b
    PG:172 org.chromium.android_webview.AwContentsClientBridge.shouldOverrideUrlLoading
    PG:-2 org.chromium.base.SystemMessageHandler.nativeDoRunLoopOnce
    PG:9 org.chromium.base.SystemMessageHandler.handleMessage
    Handler.java:105 android.os.Handler.dispatchMessage
    Looper.java:156 android.os.Looper.loop
    ActivityThread.java:6523 android.app.ActivityThread.main
    Method.java:-2 java.lang.reflect.Method.invoke
    ZygoteInit.java:942 com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run
    ZygoteInit.java:832 com.android.internal.os.ZygoteInit.main

@andrewlewis
Copy link
Collaborator

@brinkkemper This suggests IMA is calling loadAd after we've called adsManager.destroy(). I'll need to investigate a bit further to see how that can happen. With the workaround in #4231 this shouldn't cause a crash any more.

@fpittersacc Using a flag should work. I'll plan to make a fix, either using a flag or by checking for queued attach messages in detach, and removing them if present. (The latter is a bit more code but would work for all AdsLoader implementations.)

ojw28 pushed a commit that referenced this issue Jun 28, 2018
Issue: #3879

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=202100576
ojw28 pushed a commit that referenced this issue Jul 23, 2018
Issue: #3879

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=202100576
@google google locked and limited conversation to collaborators Nov 23, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

5 participants