From 09ef1cb126ee59dcb4eacfc3a8afefcb0f991945 Mon Sep 17 00:00:00 2001 From: Jack Yu Date: Sun, 18 Aug 2019 18:51:30 -0700 Subject: [PATCH] Fixed null callback issue Fixed the vendor data/network service non-responsive issue. The callback binder passed from frameworks might be GC'd so that vendor data/network service skipped calling the callback. This eventually caused data connection state machine messed up. Fixed by turning the weak refernce to the callback into a strong reference. This ensure the binder alive when vendor service needs to invoke the callback. Test: Telephony sanity tests Bug: 139076980 Change-Id: Ica0b7b810ffd5416ffd1b2b61f7ebc4af0dcb8ce --- .../android/telephony/NetworkServiceCallback.java | 12 +++--- .../telephony/data/DataServiceCallback.java | 47 ++++++++++++---------- 2 files changed, 31 insertions(+), 28 deletions(-) diff --git a/telephony/java/android/telephony/NetworkServiceCallback.java b/telephony/java/android/telephony/NetworkServiceCallback.java index 1c64bcd28966..89b96654451e 100644 --- a/telephony/java/android/telephony/NetworkServiceCallback.java +++ b/telephony/java/android/telephony/NetworkServiceCallback.java @@ -24,7 +24,6 @@ import android.telephony.NetworkService.NetworkServiceProvider; import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; -import java.lang.ref.WeakReference; /** * Network service callback. Object of this class is passed to NetworkServiceProvider upon @@ -61,11 +60,11 @@ public class NetworkServiceCallback { /** Request failed */ public static final int RESULT_ERROR_FAILED = 5; - private final WeakReference mCallback; + private final INetworkServiceCallback mCallback; /** @hide */ public NetworkServiceCallback(INetworkServiceCallback callback) { - mCallback = new WeakReference<>(callback); + mCallback = callback; } /** @@ -78,15 +77,14 @@ public class NetworkServiceCallback { */ public void onRequestNetworkRegistrationInfoComplete(int result, @Nullable NetworkRegistrationInfo state) { - INetworkServiceCallback callback = mCallback.get(); - if (callback != null) { + if (mCallback != null) { try { - callback.onRequestNetworkRegistrationInfoComplete(result, state); + mCallback.onRequestNetworkRegistrationInfoComplete(result, state); } catch (RemoteException e) { Rlog.e(mTag, "Failed to onRequestNetworkRegistrationInfoComplete on the remote"); } } else { - Rlog.e(mTag, "Weak reference of callback is null."); + Rlog.e(mTag, "onRequestNetworkRegistrationInfoComplete callback is null."); } } } \ No newline at end of file diff --git a/telephony/java/android/telephony/data/DataServiceCallback.java b/telephony/java/android/telephony/data/DataServiceCallback.java index 89d30c0d4373..11dc78a611ff 100644 --- a/telephony/java/android/telephony/data/DataServiceCallback.java +++ b/telephony/java/android/telephony/data/DataServiceCallback.java @@ -27,7 +27,6 @@ import android.telephony.data.DataService.DataServiceProvider; import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; -import java.lang.ref.WeakReference; import java.util.List; /** @@ -64,11 +63,11 @@ public class DataServiceCallback { /** Request sent in illegal state */ public static final int RESULT_ERROR_ILLEGAL_STATE = 4; - private final WeakReference mCallback; + private final IDataServiceCallback mCallback; /** @hide */ public DataServiceCallback(IDataServiceCallback callback) { - mCallback = new WeakReference<>(callback); + mCallback = callback; } /** @@ -80,14 +79,15 @@ public class DataServiceCallback { */ public void onSetupDataCallComplete(@ResultCode int result, @Nullable DataCallResponse response) { - IDataServiceCallback callback = mCallback.get(); - if (callback != null) { + if (mCallback != null) { try { if (DBG) Rlog.d(TAG, "onSetupDataCallComplete"); - callback.onSetupDataCallComplete(result, response); + mCallback.onSetupDataCallComplete(result, response); } catch (RemoteException e) { Rlog.e(TAG, "Failed to onSetupDataCallComplete on the remote"); } + } else { + Rlog.e(TAG, "onSetupDataCallComplete: callback is null!"); } } @@ -98,14 +98,15 @@ public class DataServiceCallback { * @param result The result code. Must be one of the {@link ResultCode}. */ public void onDeactivateDataCallComplete(@ResultCode int result) { - IDataServiceCallback callback = mCallback.get(); - if (callback != null) { + if (mCallback != null) { try { if (DBG) Rlog.d(TAG, "onDeactivateDataCallComplete"); - callback.onDeactivateDataCallComplete(result); + mCallback.onDeactivateDataCallComplete(result); } catch (RemoteException e) { Rlog.e(TAG, "Failed to onDeactivateDataCallComplete on the remote"); } + } else { + Rlog.e(TAG, "onDeactivateDataCallComplete: callback is null!"); } } @@ -116,13 +117,14 @@ public class DataServiceCallback { * @param result The result code. Must be one of the {@link ResultCode}. */ public void onSetInitialAttachApnComplete(@ResultCode int result) { - IDataServiceCallback callback = mCallback.get(); - if (callback != null) { + if (mCallback != null) { try { - callback.onSetInitialAttachApnComplete(result); + mCallback.onSetInitialAttachApnComplete(result); } catch (RemoteException e) { Rlog.e(TAG, "Failed to onSetInitialAttachApnComplete on the remote"); } + } else { + Rlog.e(TAG, "onSetInitialAttachApnComplete: callback is null!"); } } @@ -133,13 +135,14 @@ public class DataServiceCallback { * @param result The result code. Must be one of the {@link ResultCode}. */ public void onSetDataProfileComplete(@ResultCode int result) { - IDataServiceCallback callback = mCallback.get(); - if (callback != null) { + if (mCallback != null) { try { - callback.onSetDataProfileComplete(result); + mCallback.onSetDataProfileComplete(result); } catch (RemoteException e) { Rlog.e(TAG, "Failed to onSetDataProfileComplete on the remote"); } + } else { + Rlog.e(TAG, "onSetDataProfileComplete: callback is null!"); } } @@ -153,13 +156,14 @@ public class DataServiceCallback { */ public void onRequestDataCallListComplete(@ResultCode int result, @NonNull List dataCallList) { - IDataServiceCallback callback = mCallback.get(); - if (callback != null) { + if (mCallback != null) { try { - callback.onRequestDataCallListComplete(result, dataCallList); + mCallback.onRequestDataCallListComplete(result, dataCallList); } catch (RemoteException e) { Rlog.e(TAG, "Failed to onRequestDataCallListComplete on the remote"); } + } else { + Rlog.e(TAG, "onRequestDataCallListComplete: callback is null!"); } } @@ -170,14 +174,15 @@ public class DataServiceCallback { * @param dataCallList List of the current active data connection. */ public void onDataCallListChanged(@NonNull List dataCallList) { - IDataServiceCallback callback = mCallback.get(); - if (callback != null) { + if (mCallback != null) { try { if (DBG) Rlog.d(TAG, "onDataCallListChanged"); - callback.onDataCallListChanged(dataCallList); + mCallback.onDataCallListChanged(dataCallList); } catch (RemoteException e) { Rlog.e(TAG, "Failed to onDataCallListChanged on the remote"); } + } else { + Rlog.e(TAG, "onDataCallListChanged: callback is null!"); } } } -- 2.11.0