[🐛] Memory Leak Issue with react-native-google-mobile-ads Library

[Vukasin2212]

What happened?

I am encountering a memory leak problem that I suspect is caused by improper destruction on the library side. When the screen with banners is not focused, I remove the banners using the following approach:

return isFocused ? ( <GAMBannerAd unitId={id} sizes={sizes} onAdFailedToLoad={onAdFailedToLoad} onAdLoaded={onAdLoaded} requestOptions={{ requestNonPersonalizedAdsOnly: true, }} /> ) : null;

The banner disappears as expected, but it seems to continue running in the background. Even when I navigate to the next page and return to the page with banners, I experience a memory leak, presumably because it endlessly creates new banners without properly destroying the old ones. I haven't noticed a capability to destroy a banner before removing it from the render. Are you familiar with this issue?

Steps to Reproduce:
Navigate to a screen with banners.
Remove the banners when the screen is not focused.
Navigate to another screen.
Return to the screen with banners.
Expected Behavior:
The banners should be properly destroyed when removed from the render, preventing any memory leaks.

Additional Information:
Any insights or guidance on resolving this issue would be greatly appreciated. Thank you!

Platforms

Android and iOS

React Native Info

Library Version: react-native-google-mobile-ads@12.6.0

System:
OS: macOS 13.4
CPU: (10) arm64 Apple M1 Pro
Memory: 75.72 MB / 16.00 GB
Shell: 5.9 - /bin/zsh
Binaries:
Node: 19.0.1
Yarn: 1.22.18
npm: 8.19.2
Watchman: 2023.10.02.00
Managers:
CocoaPods: 1.14.3
SDKs:
iOS SDK:
Platforms: DriverKit 22.4, iOS 16.4, macOS 13.3, tvOS 16.4, watchOS 9.4
IDEs:
Android Studio: 2022.1 AI-221.6008.13.2211.9619390
Xcode: 14.3.1/14E300c
Languages:
Java: 11.0.11
npmPackages:
react: 18.2.0
react-native: 0.71.8

Are your using Typescript?

• My project is using Typescript

package.json

{
  "name": "p-app",
  "version": "1.0.0",
  "private": true,
  "scripts": {
    "start": "react-native start",
    "android": "react-native run-android",
    "ios": "react-native run-ios",
    "test": "jest",
    "lint": "eslint ."
  },
  "dependencies": {
    "react": "18.2.0",
    "react-native": "0.71.8",
    "react-native-google-mobile-ads": "12.6.0"
  },
  "devDependencies": {
  "@babel/core": "7.20.2",
    "@babel/plugin-proposal-private-methods": "7.18.6",
    "@babel/plugin-transform-flow-strip-types": "7.21.0",
    "@babel/preset-env": "7.20.2",
    "@babel/runtime": "7.20.0",
    "@react-native-community/eslint-config": "3.2.0",
    "babel-jest": "29.2.1",
    "eslint": "8.19.0",
    "jest": "29.2.1",
    "metro-react-native-babel-preset": "0.73.9",
    "react-test-renderer": "18.2.0"
  },
  "jest": {
    "preset": "react-native"
  }
}

app.json

{
  "name": "p-app",
  "displayName": "p-app",
  "react-native-google-mobile-ads": {
    "android_app_id": "ca-app-pub-xxx",
    "ios_app_id": "ca-app-pub-xxx"
  }
}

ios/Podfile

No response

android/build.gradle

No response

android/app/build.gradle

No response

android/settings.gradle

No response

AndroidManifest.xml

No response

[dylancom]

Haven't noticed this in our demo app.
On the Unity side there seems to be a destroy method: https://developers.google.com/admob/unity/banner#destroy_the_banner_view
What clean up method are we missing?
https://developers.google.com/admob/ios/banner
https://developers.google.com/admob/android/banner

[tominou]

I have the same issue. Memory and CPU leak leads to the app crashing after navigating to multiple screens (displaying a banner) back and forth. When I remove the banner, the issue vanishes.

I use react-navigation and I notice the issue on iOS. I didn't try on Android.

[dylancom]

Please test if #508 fixes it.

[tominou]

Sorry I do not think it fixes it.
The number of active threads keep rising as well as CPU usage (on iOS).

[dylancom]

@Vukasin2212 does the PR solve the memory leaks?

[tominou]

It seems to fix the issue on iOS release mode.
Android seems fine also. Anyone else can confirm?

[mikehardy]

🎉 This issue has been resolved in version 12.7.1 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

[Vukasin2212]

Thank you, I will check and then provide feedback.

[sedatb]

The lag problem has increased on the iOS side. I use latest version (v12.10.0). The app slows down, gets hot and crashes. iOS side is fine in older versions. Can you check this problem @mikehardy. Regards

[mikehardy]

No I can't sorry, no available volunteer time to profile things for other people. You can use the xcode profiler I believe?

[sedatb]

I also encountered the same problem. There is a event handler added in recent updates. I think this is due to the event handler. When I remove that event handler, the problem is solved. Could it be related to this? @mikehardy

remove this event handler in RNGoogleMobileAdsBannerView.mm

_banner.paidEventHandler = ^(GADAdValue *_Nonnull value) {
   std::dynamic_pointer_cast<const facebook::react::RNGoogleMobileAdsBannerViewEventEmitter>(
       _eventEmitter)
       ->onNativeEvent(facebook::react::RNGoogleMobileAdsBannerViewEventEmitter::OnNativeEvent {
         .type = "onPaid", .value = value.value.doubleValue,
         .precision = @(value.precision).doubleValue, .currency = value.currencyCode.UTF8String
       });
 };

And removed this method in RNGoogleMobileAdsFullScreenAd.swift

let paidEventHandler = {(value: GADAdValue) in
self.sendAdEvent(
  "paid",
  requestId: requestId,
  adUnitId: adUnitId,
  error: nil,
  data: [
    "value": value.value,
    "precision": value.precision.rawValue,
    "currency": value.currencyCode,
  ]
);

};

(ad as? GADRewardedAd)?.paidEventHandler = paidEventHandler;
(ad as? GADRewardedInterstitialAd)?.paidEventHandler = paidEventHandler;
(ad as? GADInterstitialAd)?.paidEventHandler = paidEventHandler;
(ad as? GADAppOpenAd)?.paidEventHandler = paidEventHandler;

[dylancom]

Tagging @birdofpreyru here who built the feature.

[birdofpreyru]

Hey @dylancom , @sedatb ,

It reads like my additions to iOS code to support impression-level ad revenue events introduced a circular dependency between objects allocated in memory, preventing the garbage collector to drop them. @sedatb have you checked that removing both of these handlers is necessary, or just removing the second of them is enough?

[sedatb]

Hi @birdofpreyru,
I'm not sure, i didnt check it but I use more than one banner in my application, I do not use fullscreen ads. Therefore, I think the problem is in the event handler in RNGoogleMobileAdsBannerView.mm

[dylancom]

Could you provide a demo which demonstrates the lag?
If it's the paidEventHandler it could be fixed by only adding the handler when an onPaid prop was added to the banner.
Happy to merge a PR...

[birdofpreyru]

Ok, I've confirmed the problem... me not knowing Objective-C well enough (or, I'd say, bad software engineering by Apple making it too easy to make such error):

https://developer.apple.com/library/archive/documentation/Cocoa/Conceptual/ProgrammingWithObjectiveC/WorkingwithBlocks/WorkingwithBlocks.html

I am on fixing it.

[birdofpreyru]

well... @dylancom @sedatb here is what I got...

The following handler is only relevant to iOS / New RN Arch. No matter whether I comment it or not, the banner views seem to never deallocate (checked by adding a log message to the dealloc() method, and running the example app I never see it printing either way). Perhaps there some other strong reference cycles preventing the deallocation, or I am doing something wrong... either way can't look more into it now, as I am not yet using old arch on ios anyway in my projects.

react-native-google-mobile-ads/ios/RNGoogleMobileAds/RNGoogleMobileAdsBannerView.mm

Lines 119 to 126 in d438bf2

	_banner.paidEventHandler = ^(GADAdValue *_Nonnull value) {
	std::dynamic_pointer_cast<const facebook::react::RNGoogleMobileAdsBannerViewEventEmitter>(
	_eventEmitter)
	->onNativeEvent(facebook::react::RNGoogleMobileAdsBannerViewEventEmitter::OnNativeEvent {
	.type = "onPaid", .value = value.value.doubleValue,
	.precision = @(value.precision).doubleValue, .currency = value.currencyCode.UTF8String
	});
	};

The similar handler in the iOS / Old RN Arch banners code indeed makes the difference between allowing and not allowing the banner components to deallocate (checked in the similar way mentioned above), because the mistake explained in my previous message. I've submitted PR #565 fixing it — please merge it.

react-native-google-mobile-ads/ios/RNGoogleMobileAds/RNGoogleMobileAdsBannerComponent.m

Lines 120 to 127 in d438bf2

	_banner.paidEventHandler = ^(GADAdValue *_Nonnull value) {
	[self sendEvent:@"onPaid"
	payload:@{
	@"value" : value.value,
	@"precision" : @(value.precision),
	@"currency" : value.currencyCode,
	}];
	};

For all sorts of interstitials / full screen ads I am not sure... The deinit() method does not seem to be ever called, no matter whether I comment out the handler or not... needs to be further investigated some time later.

react-native-google-mobile-ads/ios/RNGoogleMobileAds/RNGoogleMobileAdsFullScreenAd.swift

Lines 95 to 112 in d438bf2

	let paidEventHandler = {(value: GADAdValue) in
	self.sendAdEvent(
	"paid",
	requestId: requestId,
	adUnitId: adUnitId,
	error: nil,
	data: [
	"value": value.value,
	"precision": value.precision.rawValue,
	"currency": value.currencyCode,
	]
	);
	};
	(ad as? GADRewardedAd)?.paidEventHandler = paidEventHandler;
	(ad as? GADRewardedInterstitialAd)?.paidEventHandler = paidEventHandler;
	(ad as? GADInterstitialAd)?.paidEventHandler = paidEventHandler;
	(ad as? GADAppOpenAd)?.paidEventHandler = paidEventHandler;

[birdofpreyru]

For record — the memory leak for banners on iOS / Old Arch is fixed in v13.2.1; I am not yet sure the same leak does not happen with New Arch even in v13.2.1, and I'd double-check if it is a problem for interstitials.

[github-actions]

Hello 👋, to help manage issues we automatically close stale issues.

This issue has been automatically marked as stale because it has not had activity for quite some time.Has this issue been fixed, or does it still require attention?

This issue will be closed in 15 days if no further activity occurs.

Thank you for your contributions.

[birdofpreyru]

Let's keep it open — I'd double-check there is no memory leak on the iOS / new arch, with the latest code version.