larpon / QtFirebase

An effort to bring Google's Firebase C++ API to Qt + QML
MIT License
283 stars 83 forks source link

Crashes while ad loading on android #90

Open GaRaOne opened 5 years ago

GaRaOne commented 5 years ago

We have serveral crash reports on android in our last release... mostly while loading of banner and rewared ads... interstitials seems to be stable instead.

do you have any idea?

we shipped with:

QtFirebaseAdMobBanner::loadInternal


pid: 0, tid: 0 >>> de.bitspree.pico.islands <<<

backtrace:

00 pc 0000000000049b58 /system/lib/libc.so (tgkill+12)

01 pc 00000000000472d3 /system/lib/libc.so (pthread_kill+34)

02 pc 000000000001d575 /system/lib/libc.so (raise+10)

03 pc 00000000000190c1 /system/lib/libc.so (__libc_android_abort+34)

04 pc 0000000000017124 /system/lib/libc.so (abort+4)

05 pc 000000000031b945 /system/lib/libart.so (_ZN3art7Runtime5AbortEPKc+328)

06 pc 00000000000b561d /system/lib/libart.so (_ZN3art10LogMessageD2Ev+1132)

07 pc 00000000001bcc23 /system/lib/libart.so (_ZN3art22IndirectReferenceTable3AddEjPNS_6mirror6ObjectE+194)

08 pc 0000000000265e09 /system/lib/libart.so (_ZN3art3JNI17CallObjectMethodVEP7_JNIEnvP8_jobjectP10_jmethodIDSt9__va_list+456)

09 pc 000000000011c273 /data/app/de.bitspree.pico.islands-1/lib/arm/libpico-islands.so (_ZN7_JNIEnv16CallObjectMethodEP8_jobjectP10_jmethodIDz+42)

10 pc 000000000011fb85 /data/app/de.bitspree.pico.islands-1/lib/arm/libpico-islands.so (_ZN8firebase5admob18AdRequestConverterC1ENS0_9AdRequestE+640)

11 pc 000000000011bf77 /data/app/de.bitspree.pico.islands-1/lib/arm/libpico-islands.so (_ZN8firebase5admob8internal25BannerViewInternalAndroid6LoadAdERKNS0_9AdRequestE+78)

12 pc 000000000011d995 /data/app/de.bitspree.pico.islands-1/lib/arm/libpico-islands.so (_ZN8firebase5admob10BannerView6LoadAdERKNS0_9AdRequestE+34)

13 pc 00000000000877cd /data/app/de.bitspree.pico.islands-1/lib/arm/libpico-islands.so (_ZN21QtFirebaseAdMobBanner12loadInternalEv+40)

14 pc 0000000000085afb /data/app/de.bitspree.pico.islands-1/lib/arm/libpico-islands.so (_ZN19QtFirebaseAdMobBase4loadEv+224)

15 pc 000000000010f2df /data/app/de.bitspree.pico.islands-1/lib/arm/libpico-islands.so

16 pc 000000000010f685 /data/app/de.bitspree.pico.islands-1/lib/arm/libpico-islands.so (_ZN19QtFirebaseAdMobBase11qt_metacallEN11QMetaObject4CallEiPPv+30)

17 pc 000000000010f6d9 /data/app/de.bitspree.pico.islands-1/lib/arm/libpico-islands.so (_ZN25QtFirebaseAdMobBannerBase11qt_metacallEN11QMetaObject4CallEiPPv+8)

18 pc 00000000001081b5 /data/app/de.bitspree.pico.islands-1/lib/arm/libQt5Qml.so (_ZN17QQmlVMEMetaObject8metaCallEP7QObjectN11QMetaObject4CallEiPPv+2584)


QtFirebaseAdMobRewardedVideoAd::loadInternal


pid: 0, tid: 0 >>> de.bitspree.pico.islands <<<

backtrace:

00 pc 000000000004a1c0 /system/lib/libc.so (tgkill+12)

01 pc 0000000000047953 /system/lib/libc.so (pthread_kill+34)

02 pc 000000000001d955 /system/lib/libc.so (raise+10)

03 pc 00000000000194a1 /system/lib/libc.so (__libc_android_abort+34)

04 pc 00000000000170e8 /system/lib/libc.so (abort+4)

05 pc 000000000031b5c5 /system/lib/libart.so (_ZN3art7Runtime5AbortEv+252)

06 pc 00000000000b4d47 /system/lib/libart.so (_ZN3art10LogMessageD2Ev+866)

07 pc 00000000001bd02f /system/lib/libart.so (_ZN3art22IndirectReferenceTable3AddEjPNS_6mirror6ObjectE+194)

08 pc 000000000027cc99 /system/lib/libart.so (_ZN3art3JNI12NewStringUTFEP7_JNIEnvPKc+424)

09 pc 000000000011f98b /data/app/de.bitspree.pico.islands-1/lib/arm/libpico-islands.so (_ZN8firebase5admob18AdRequestConverterC1ENS0_9AdRequestE+134)

10 pc 000000000011d481 /data/app/de.bitspree.pico.islands-1/lib/arm/libpico-islands.so (_ZN8firebase5admob14rewarded_video8internal28RewardedVideoInternalAndroid6LoadAdEPKcRKNS0_9AdRequestE+96)

11 pc 000000000011eb0d /data/app/de.bitspree.pico.islands-1/lib/arm/libpico-islands.so (_ZN8firebase5admob14rewarded_video6LoadAdEPKcRKNS0_9AdRequestE+40)

12 pc 0000000000087885 /data/app/de.bitspree.pico.islands-1/lib/arm/libpico-islands.so (_ZN30QtFirebaseAdMobRewardedVideoAd12loadInternalEv+40)

13 pc 0000000000085afb /data/app/de.bitspree.pico.islands-1/lib/arm/libpico-islands.so (_ZN19QtFirebaseAdMobBase4loadEv+224)

14 pc 000000000010f2df /data/app/de.bitspree.pico.islands-1/lib/arm/libpico-islands.so

15 pc 000000000010f685 /data/app/de.bitspree.pico.islands-1/lib/arm/libpico-islands.so (_ZN19QtFirebaseAdMobBase11qt_metacallEN11QMetaObject4CallEiPPv+30)

16 pc 000000000010f781 /data/app/de.bitspree.pico.islands-1/lib/arm/libpico-islands.so (_ZN30QtFirebaseAdMobRewardedVideoAd11qt_metacallEN11QMetaObject4CallEiPPv+8)

17 pc 00000000001081b5 /data/app/de.bitspree.pico.islands-1/lib/arm/libQt5Qml.so (_ZN17QQmlVMEMetaObject8metaCallEP7QObjectN11QMetaObject4CallEiPPv+2584)

larpon commented 5 years ago

Hmm... Someone added thread safety recently for Qt versions > 5.10 maybe it's a result of those changes? I can't dig out the commit right now - but it's fairly recent. Maybe that's the culprit?

larpon commented 5 years ago

@GaRaOne - this PR https://github.com/Larpon/QtFirebase/pull/82

GaRaOne commented 5 years ago

It seems to be a fundamental problem of the Admob implementation in QtFirebase. The use of callbacks across threads and the storage of payload in the objects, which can be destroyed at any time by QML.

I try to read in the code the next few days. When I get to that, I completely change the code to the polling mechanisms of firebase_cpp.

GaRaOne commented 5 years ago

greets from germany

There is a first iteration of the refactoring: https://github.com/GaRaOne/QtFirebase/blob/admob-refactoring/src/qtfirebaseadmob.h https://github.com/GaRaOne/QtFirebase/blob/admob-refactoring/src/qtfirebaseadmob.cpp

We still testing the branch, so it is not bug free. We also currently have no chance to test it on iOS in the wild.

Take a look at the changes and tell me how we go on from here?

larpon commented 5 years ago

@GaRaOne - thanks for your efforts! I'll take a look as soon as possible. I'm drowning in work ATM

larpon commented 5 years ago

... on a quick side note: I don't think I'll do the merges manually no matter what. I haven't looked at you code yet - but I know we moved away from polling via the event-loop some time ago - which makes it unlikely that we would bring it back. Also so far you're the only one to report so severe crashes. Could it be you're using it in exotic ways?

When I first wrote the AdMob module it was made to support primarily QML - so it's use was designed to follow the QML engine's garbage collection. I haven't had any crashes recently in any of my live apps using it - that's why I'm questioning the crashes :slightly_smiling_face:

GaRaOne commented 5 years ago

When I first wrote the AdMob module it was made to support primarily QML - so it's use was designed to follow the QML engine's garbage collection. I haven't had any crashes recently in any of my live apps using it - that's why I'm questioning the crashes

The Qml flow is the cause for the crashes. firebase_cpp assume static c strings in all its examples. Qml/QtFirebase use dynamic allocated strings. you tried to solve this by caching the strings in the Request-Object. But at the point an AdmobBanner, AdmobInterstitial or AdmobRewardedAd gets destructed, all the this strings/pointer get undefined. Same if you change a Request Property.

Also there is a flaw with the callbacks. If a callback is processed after the destruction of an AdmobXxx instance, the whole app crashes. I try to do no logic in the Listener-Callbacks (OnBoundingBoxChanged is the only left) to address this Problem.

We have a trigger in the App which activates/deactivates all Advertising by QmlLoaders, if the user purchase an InApp-Item ... with the master branch it crashes in QtFirebase/firebase_cpp nearly every time.

Just look at it and give it a try :) ...