From fb62e63f03b1728f3ada5ae9c39594803a41d441 Mon Sep 17 00:00:00 2001 From: Jono Sligh Date: Tue, 19 Sep 2023 11:55:26 -0500 Subject: [PATCH 1/5] Response Callback could be null so surrounding with Null checks. --- .../rendering/networking/modelcontrollers/BidRequester.java | 4 +++- .../rendering/networking/modelcontrollers/Requester.java | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/PrebidMobile/PrebidMobile-core/src/main/java/org/prebid/mobile/rendering/networking/modelcontrollers/BidRequester.java b/PrebidMobile/PrebidMobile-core/src/main/java/org/prebid/mobile/rendering/networking/modelcontrollers/BidRequester.java index 468e1f7cd..00c0a8e15 100644 --- a/PrebidMobile/PrebidMobile-core/src/main/java/org/prebid/mobile/rendering/networking/modelcontrollers/BidRequester.java +++ b/PrebidMobile/PrebidMobile-core/src/main/java/org/prebid/mobile/rendering/networking/modelcontrollers/BidRequester.java @@ -36,7 +36,9 @@ public BidRequester(AdUnitConfiguration config, AdRequestInput adRequestInput, R @Override public void startAdRequest() { if (TextUtils.isEmpty(adConfiguration.getConfigId())) { - adResponseCallBack.onError("No configuration id specified.", 0); + if (adResponseCallBack != null) { + adResponseCallBack.onError("No configuration id specified.", 0); + } return; } diff --git a/PrebidMobile/PrebidMobile-core/src/main/java/org/prebid/mobile/rendering/networking/modelcontrollers/Requester.java b/PrebidMobile/PrebidMobile-core/src/main/java/org/prebid/mobile/rendering/networking/modelcontrollers/Requester.java index 962e67a2c..1508a4cd0 100644 --- a/PrebidMobile/PrebidMobile-core/src/main/java/org/prebid/mobile/rendering/networking/modelcontrollers/Requester.java +++ b/PrebidMobile/PrebidMobile-core/src/main/java/org/prebid/mobile/rendering/networking/modelcontrollers/Requester.java @@ -149,7 +149,9 @@ public void adIdFetchFailure() { private void sendAdException(String logMsg, String exceptionMsg) { LogUtil.warning(TAG, logMsg); AdException adException = new AdException(AdException.INIT_ERROR, exceptionMsg); - adResponseCallBack.onErrorWithException(adException, 0); + if (adResponseCallBack != null) { + adResponseCallBack.onErrorWithException(adException, 0); + } } protected void makeAdRequest() { From 4bf67102b33ed1a93ac18a73d70410fc2eb3fae0 Mon Sep 17 00:00:00 2001 From: Jono Sligh Date: Wed, 20 Sep 2023 10:38:00 -0500 Subject: [PATCH 2/5] Adding an isDestroyed flag as well as a warning. Leaving the null check for the rare case where it is null. --- .../networking/modelcontrollers/Requester.java | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/PrebidMobile/PrebidMobile-core/src/main/java/org/prebid/mobile/rendering/networking/modelcontrollers/Requester.java b/PrebidMobile/PrebidMobile-core/src/main/java/org/prebid/mobile/rendering/networking/modelcontrollers/Requester.java index 1508a4cd0..fc5bfae1f 100644 --- a/PrebidMobile/PrebidMobile-core/src/main/java/org/prebid/mobile/rendering/networking/modelcontrollers/Requester.java +++ b/PrebidMobile/PrebidMobile-core/src/main/java/org/prebid/mobile/rendering/networking/modelcontrollers/Requester.java @@ -59,6 +59,7 @@ public abstract class Requester { protected URLBuilder urlBuilder; protected ResponseHandler adResponseCallBack; protected BaseNetworkTask networkTask; + protected Boolean isDestroyed = false; Requester( AdUnitConfiguration config, @@ -87,6 +88,7 @@ public void destroy() { } networkTask = null; adResponseCallBack = null; + isDestroyed = true; } protected List getParameterBuilders() { @@ -129,13 +131,21 @@ protected void getAdId() { @Override public void adIdFetchCompletion() { LogUtil.info(TAG, "Advertising id was received"); - makeAdRequest(); + if (isDestroyed) { + LogUtil.warning(TAG, "Attempted to make ad request after requester was destroyed"); + } else { + makeAdRequest(); + } } @Override public void adIdFetchFailure() { LogUtil.warning(TAG, "Can't get advertising id"); - makeAdRequest(); + if (isDestroyed) { + LogUtil.warning(TAG, "Attempted to make ad request after requester was destroyed"); + } else { + makeAdRequest(); + } } }); } else { From a157b79dab3c711f21cee1a66926725a163e9d13 Mon Sep 17 00:00:00 2001 From: Jono Sligh Date: Wed, 20 Sep 2023 15:09:25 -0500 Subject: [PATCH 3/5] Removed destroyed state and included a weakReference. --- .../networking/modelcontrollers/Requester.java | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/PrebidMobile/PrebidMobile-core/src/main/java/org/prebid/mobile/rendering/networking/modelcontrollers/Requester.java b/PrebidMobile/PrebidMobile-core/src/main/java/org/prebid/mobile/rendering/networking/modelcontrollers/Requester.java index fc5bfae1f..85a16f7eb 100644 --- a/PrebidMobile/PrebidMobile-core/src/main/java/org/prebid/mobile/rendering/networking/modelcontrollers/Requester.java +++ b/PrebidMobile/PrebidMobile-core/src/main/java/org/prebid/mobile/rendering/networking/modelcontrollers/Requester.java @@ -47,6 +47,7 @@ import org.prebid.mobile.rendering.utils.helpers.AppInfoManager; import org.prebid.mobile.rendering.utils.helpers.ExternalViewerUtils; +import java.lang.ref.WeakReference; import java.util.ArrayList; import java.util.List; @@ -59,7 +60,6 @@ public abstract class Requester { protected URLBuilder urlBuilder; protected ResponseHandler adResponseCallBack; protected BaseNetworkTask networkTask; - protected Boolean isDestroyed = false; Requester( AdUnitConfiguration config, @@ -88,7 +88,6 @@ public void destroy() { } networkTask = null; adResponseCallBack = null; - isDestroyed = true; } protected List getParameterBuilders() { @@ -126,25 +125,22 @@ protected void getAdId() { } UserConsentManager userConsentManager = ManagersResolver.getInstance().getUserConsentManager(); + WeakReference weakRequester = new WeakReference<>(this); if (userConsentManager.canAccessDeviceData()) { AdIdManager.initAdId(context, new AdIdFetchListener() { @Override public void adIdFetchCompletion() { LogUtil.info(TAG, "Advertising id was received"); - if (isDestroyed) { - LogUtil.warning(TAG, "Attempted to make ad request after requester was destroyed"); - } else { - makeAdRequest(); + if (weakRequester.get() != null) { + weakRequester.get().makeAdRequest(); } } @Override public void adIdFetchFailure() { LogUtil.warning(TAG, "Can't get advertising id"); - if (isDestroyed) { - LogUtil.warning(TAG, "Attempted to make ad request after requester was destroyed"); - } else { - makeAdRequest(); + if (weakRequester.get() != null) { + weakRequester.get().makeAdRequest(); } } }); From 5d31a144d248efc47fbe795cda53948ac00b8397 Mon Sep 17 00:00:00 2001 From: Jono Sligh Date: Thu, 21 Sep 2023 09:35:38 -0500 Subject: [PATCH 4/5] Implemented proposed fix similar to the way Network Task is handled in the same class. --- .../networking/modelcontrollers/Requester.java | 16 ++++++++-------- .../rendering/utils/helpers/AdIdManager.java | 6 ++++-- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/PrebidMobile/PrebidMobile-core/src/main/java/org/prebid/mobile/rendering/networking/modelcontrollers/Requester.java b/PrebidMobile/PrebidMobile-core/src/main/java/org/prebid/mobile/rendering/networking/modelcontrollers/Requester.java index 85a16f7eb..ca26d38b2 100644 --- a/PrebidMobile/PrebidMobile-core/src/main/java/org/prebid/mobile/rendering/networking/modelcontrollers/Requester.java +++ b/PrebidMobile/PrebidMobile-core/src/main/java/org/prebid/mobile/rendering/networking/modelcontrollers/Requester.java @@ -60,6 +60,7 @@ public abstract class Requester { protected URLBuilder urlBuilder; protected ResponseHandler adResponseCallBack; protected BaseNetworkTask networkTask; + protected AdIdManager.FetchAdIdInfoTask fetchAdIdInfoTask; Requester( AdUnitConfiguration config, @@ -88,6 +89,10 @@ public void destroy() { } networkTask = null; adResponseCallBack = null; + if (fetchAdIdInfoTask != null) { + fetchAdIdInfoTask.cancel(true); + } + fetchAdIdInfoTask = null; } protected List getParameterBuilders() { @@ -125,23 +130,18 @@ protected void getAdId() { } UserConsentManager userConsentManager = ManagersResolver.getInstance().getUserConsentManager(); - WeakReference weakRequester = new WeakReference<>(this); if (userConsentManager.canAccessDeviceData()) { - AdIdManager.initAdId(context, new AdIdFetchListener() { + fetchAdIdInfoTask = AdIdManager.initAdId(context, new AdIdFetchListener() { @Override public void adIdFetchCompletion() { LogUtil.info(TAG, "Advertising id was received"); - if (weakRequester.get() != null) { - weakRequester.get().makeAdRequest(); - } + makeAdRequest(); } @Override public void adIdFetchFailure() { LogUtil.warning(TAG, "Can't get advertising id"); - if (weakRequester.get() != null) { - weakRequester.get().makeAdRequest(); - } + makeAdRequest(); } }); } else { diff --git a/PrebidMobile/PrebidMobile-core/src/main/java/org/prebid/mobile/rendering/utils/helpers/AdIdManager.java b/PrebidMobile/PrebidMobile-core/src/main/java/org/prebid/mobile/rendering/utils/helpers/AdIdManager.java index 523d8973a..ed5b57272 100644 --- a/PrebidMobile/PrebidMobile-core/src/main/java/org/prebid/mobile/rendering/utils/helpers/AdIdManager.java +++ b/PrebidMobile/PrebidMobile-core/src/main/java/org/prebid/mobile/rendering/utils/helpers/AdIdManager.java @@ -50,7 +50,7 @@ private AdIdManager() { } // Wrap method execution in try / catch to avoid crashes in runtime if publisher didn't include identifier dependencies - public static void initAdId(final Context context, final AdIdFetchListener listener) { + public static FetchAdIdInfoTask initAdId(final Context context, final AdIdFetchListener listener) { try { GoogleApiAvailability apiAvailability = GoogleApiAvailability.getInstance(); int resultCode = apiAvailability.isGooglePlayServicesAvailable(context); @@ -67,6 +67,7 @@ public static void initAdId(final Context context, final AdIdFetchListener liste listener.adIdFetchFailure(); } }, AD_ID_TIMEOUT_MS); + return getAdIdInfoTask; } else { listener.adIdFetchCompletion(); @@ -75,6 +76,7 @@ public static void initAdId(final Context context, final AdIdFetchListener liste catch (Throwable throwable) { LogUtil.error(TAG, "Failed to initAdId: " + Log.getStackTraceString(throwable) + "\nDid you add necessary dependencies?"); } + return null; } /** @@ -97,7 +99,7 @@ public static void setAdId(String adId) { sAdId = adId; } - private static class FetchAdIdInfoTask extends AsyncTask { + public static class FetchAdIdInfoTask extends AsyncTask { private final WeakReference contextWeakReference; private final AdIdFetchListener adIdFetchListener; From 53c858262194cb386b73d2b84a37e1db276e8b8f Mon Sep 17 00:00:00 2001 From: Jono Sligh Date: Tue, 26 Sep 2023 13:58:24 -0500 Subject: [PATCH 5/5] Requested change to remove race condition. --- .../mobile/rendering/networking/modelcontrollers/Requester.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/PrebidMobile/PrebidMobile-core/src/main/java/org/prebid/mobile/rendering/networking/modelcontrollers/Requester.java b/PrebidMobile/PrebidMobile-core/src/main/java/org/prebid/mobile/rendering/networking/modelcontrollers/Requester.java index ca26d38b2..d42704f73 100644 --- a/PrebidMobile/PrebidMobile-core/src/main/java/org/prebid/mobile/rendering/networking/modelcontrollers/Requester.java +++ b/PrebidMobile/PrebidMobile-core/src/main/java/org/prebid/mobile/rendering/networking/modelcontrollers/Requester.java @@ -88,10 +88,10 @@ public void destroy() { networkTask.destroy(); } networkTask = null; - adResponseCallBack = null; if (fetchAdIdInfoTask != null) { fetchAdIdInfoTask.cancel(true); } + adResponseCallBack = null; fetchAdIdInfoTask = null; }