AdMob (Android): Possible deadlock in destruction of BannerView on Android 9

[GaRaOne]

We have increased ANR in our app for some time now. When executing the destructor of ~BannerView() there seems to be a deadlock in the mutex of BannerViewInternalAndroid

It is noticeable that these ANR only occur on Android 9.

Do you have any ideas on cause? How can we help you narrow down the problem?

We using firebase-cpp version 5.6.1

"qtMainLoopThread" prio=5 tid=14 Native
| group="main" sCount=1 dsCount=0 flags=1 obj=0x13000708 self=0xd2f08600
| sysTid=22096 nice=0 cgrp=default sched=0/0 handle=0xd2dff970
| state=S schedstat=( 9056905915 6631793329 5093 ) utm=737 stm=168 core=0 HZ=100
| stack=0xd2cfc000-0xd2cfe000 stackSize=1042KB
| held mutexes=
#00 pc 0000000000019e0c /system/lib/libc.so (syscall+28)
#1 pc 000000000001d527 /system/lib/libc.so (__futex_wait_ex(void volatile*, bool, int, bool, timespec const*)+86)
#2 pc 000000000006538b /system/lib/libc.so (NonPI::MutexLockWithTimeout(pthread_mutex_internal_t*, bool, timespec const*)+154)
#3 pc 00000000001c0f67 /data/app/de.bitspree.pico.islands-wctmlti9_uuQI6dNFFX5Hw==/lib/arm/libpico-islands.so (firebase::admob::internal::BannerViewInternalAndroid::~BannerViewInternalAndroid()+126)
#4 pc 00000000001c103d /data/app/de.bitspree.pico.islands-wctmlti9_uuQI6dNFFX5Hw==/lib/arm/libpico-islands.so (firebase::admob::internal::BannerViewInternalAndroid::~BannerViewInternalAndroid()+4)
#5 pc 00000000001c2a91 /data/app/de.bitspree.pico.islands-wctmlti9_uuQI6dNFFX5Hw==/lib/arm/libpico-islands.so (firebase::admob::BannerView::~BannerView()+24)
#6 pc 000000000014d07b /data/app/de.bitspree.pico.islands-wctmlti9_uuQI6dNFFX5Hw==/lib/arm/libpico-islands.so (Logic::FirebaseBanner::~FirebaseBanner()+278)
at org.qtproject.qt5.android.QtNative.startQtApplication (Native method)
at org.qtproject.qt5.android.QtNative$7.run (QtNative.java:374)
at org.qtproject.qt5.android.QtThread$1.run (QtThread.java:61)
at java.lang.Thread.run (Thread.java:764)

[stewartmiles]

Looking at

firebase-cpp-sdk/admob/src/android/banner_view_internal_android.cc

Line 66 in c6a326f

Destroy().OnCompletion(

it's possible to end up in this scenario if you have some code destroying the banner via BannerView::Destroy() with other code also trying to delete the class since the future returned by BannerViewInternalAndroid::InvokeNullary is racy.

The other thing that looks dodgy in here is that the completion is using a Mutex to signal another thread which undefined behavior. Really we should be using a semaphore in here.

Any chance you're code is calling Destroy() and the destructor of the BannerView from separate threads?

[GaRaOne]

We call Destroy(). However, this should be done from the same thread shortly before. Not guaranteed that Destroy is complete until the destructor ~BannerView is called.

So it would probably be best to remove the Destroy() call from our destructor ~FirebaseBanner?!

virtual ~FirebaseBanner() override
{
        LOG_INFO( "FIREBASE", "Banner::~Banner" );
        if( m_isReady )
        {
            firebase::admob::BannerView::SetListener( nullptr );

            waitForFutureCompletion(
                firebase::admob::BannerView::Hide() );

            waitForFutureCompletion(
                firebase::admob::BannerView::Destroy() );
        }
 }

static
void
waitForFutureCompletion( firebase::FutureBase future )
{
    static int count = 0;
    LOG_INFO_STATIC( "FIREBASE", "waitForFutureCompletion" );

    while( future.status() == firebase::kFutureStatusPending )
    {
        count++;

        if( count % 100 == 0 ) { LOG_INFO_STATIC( "FIREBASE", "waitForFutureCompletion 100" ); }
        if( count % 200 == 0 ) { LOG_INFO_STATIC( "FIREBASE", "waitForFutureCompletion 200" ); break; }
        QThread::msleep( 10 );
    }

    switch( future.status() )
    {
        case firebase::kFutureStatusPending : LOG_INFO_STATIC( "FIREBASE", "waitForFutureCompletion PENDING"   ); return;
        case firebase::kFutureStatusInvalid : LOG_INFO_STATIC( "FIREBASE", "waitForFutureCompletion INVALID"   ); return;
        case firebase::kFutureStatusComplete: LOG_INFO_STATIC( "FIREBASE", "waitForFutureCompletion COMPLETED" ); return;
    }
}

[stewartmiles]

Yeah it's probably worth not calling Destroy() from the destructor of your object if you're deriving from firebase::admob::BannerView. This still smells like a bug though as it looks like you're waiting up to 2s for the Destroy() operation before calling ~BannerView which should result in the next call to Destroy() doing nothing.

[GaRaOne]

Okay, then I'll close the ticket for now.

if the problem occurs again in the next release with the change, i will give feedback again.

thank you very much for the quick help.