From 594931f5d993106ec66768f2f08e8eedc47cc302 Mon Sep 17 00:00:00 2001 From: Jack Yu Date: Wed, 14 Aug 2019 02:32:17 -0700 Subject: [PATCH 1/2] Added debug messages and watchdog Added debug messages and watchdog to catch unresponsive data service issue. Bug: 139076980 Test: Manual Merged-In: I52dd126fddc1b1e8b18d5388a92aabaa3e2e48d5 Change-Id: I52dd126fddc1b1e8b18d5388a92aabaa3e2e48d5 (cherry picked from commit 36e2bfbaad3e1edcbb3ad0bf2be1e94643c38b07) --- .../java/android/telephony/data/DataServiceCallback.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/telephony/java/android/telephony/data/DataServiceCallback.java b/telephony/java/android/telephony/data/DataServiceCallback.java index 5d8d79367ba4f..89d30c0d4373c 100644 --- a/telephony/java/android/telephony/data/DataServiceCallback.java +++ b/telephony/java/android/telephony/data/DataServiceCallback.java @@ -42,6 +42,8 @@ public class DataServiceCallback { private static final String TAG = DataServiceCallback.class.getSimpleName(); + private static final boolean DBG = true; + /** * Result of data requests * @hide @@ -81,6 +83,7 @@ public class DataServiceCallback { IDataServiceCallback callback = mCallback.get(); if (callback != null) { try { + if (DBG) Rlog.d(TAG, "onSetupDataCallComplete"); callback.onSetupDataCallComplete(result, response); } catch (RemoteException e) { Rlog.e(TAG, "Failed to onSetupDataCallComplete on the remote"); @@ -98,6 +101,7 @@ public class DataServiceCallback { IDataServiceCallback callback = mCallback.get(); if (callback != null) { try { + if (DBG) Rlog.d(TAG, "onDeactivateDataCallComplete"); callback.onDeactivateDataCallComplete(result); } catch (RemoteException e) { Rlog.e(TAG, "Failed to onDeactivateDataCallComplete on the remote"); @@ -169,6 +173,7 @@ public class DataServiceCallback { IDataServiceCallback callback = mCallback.get(); if (callback != null) { try { + if (DBG) Rlog.d(TAG, "onDataCallListChanged"); callback.onDataCallListChanged(dataCallList); } catch (RemoteException e) { Rlog.e(TAG, "Failed to onDataCallListChanged on the remote"); From 7302b24f486b3e700a6b9e45a57119ecb63f0cce Mon Sep 17 00:00:00 2001 From: Jack Yu Date: Sun, 18 Aug 2019 18:51:30 -0700 Subject: [PATCH 2/2] 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 Merged-In: Ica0b7b810ffd5416ffd1b2b61f7ebc4af0dcb8ce Change-Id: Ica0b7b810ffd5416ffd1b2b61f7ebc4af0dcb8ce (cherry picked from commit 09ef1cb126ee59dcb4eacfc3a8afefcb0f991945) --- .../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 1c64bcd289661..89b96654451e9 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 89d30c0d4373c..11dc78a611ff6 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!"); } } }