Do not give WifiEntry callbacks the WifiEntry lock

Callbacks for WifiEntry should not be given the WifiEntry lock before
the callback method is run. This could result in the lock being held
while code outside of WifiEntry is run, causing deadlock scenarios.

For example:
1) WifiPickerTracker.updateWifiEntries()  acquires WifiPickerTracker
lock to update WifiEntry list.
2) The connected WifiEntry is updated and posts a callback to UI thread
3) UI thread acquires the WifiEntry lock outside of the WifiEntry
methods, runs its own updating logic. If the callback is WifiScanWorker,
it will try to call WifiPickerTracker.getWifiEntries().
4) WifiPickerTracker.getWifiEntries() is blocked on
WifiPickerTracker.updateWifiEntries().
5) WifiPickerTracker.updateWifiEntries() is blocked on the connected
WifiEntry lock, which it needs later in the method. But this is held by
WifiScanWorker, which is blocked on the WifiPickerTracker lock.

Making sure the WifiEntry lock is never held outside of its own methods
will avoid a deadlock scenario.

Bug: 190578653
Test: build
Change-Id: Ifd6922636e94ccd7388e51d3d597155b3382c91c

4 files changed, 44 insertions, 54 deletions

diff --git a/libs/WifiTrackerLib/src/com/android/wifitrackerlib/MergedCarrierEntry.java b/libs/WifiTrackerLib/src/com/android/wifitrackerlib/MergedCarrierEntry.java
index e0dc65bd9..d54999750 100644
--- a/libs/WifiTrackerLib/src/com/android/wifitrackerlib/MergedCarrierEntry.java
+++ b/libs/WifiTrackerLib/src/com/android/wifitrackerlib/MergedCarrierEntry.java
@@ -109,10 +109,9 @@ public class MergedCarrierEntry extends WifiEntry {
                 R.string.wifitrackerlib_wifi_wont_autoconnect_for_now, Toast.LENGTH_SHORT).show();
         if (mConnectCallback != null) {
             mCallbackHandler.post(() -> {
-                synchronized (this) {
-                    if (mConnectCallback != null) {
-                        mConnectCallback.onConnectResult(ConnectCallback.CONNECT_STATUS_SUCCESS);
-                    }
+                final ConnectCallback connectCallback = mConnectCallback;
+                if (connectCallback != null) {
+                    connectCallback.onConnectResult(ConnectCallback.CONNECT_STATUS_SUCCESS);
                 }
             });
         }
@@ -130,11 +129,10 @@ public class MergedCarrierEntry extends WifiEntry {
         mWifiManager.startScan();
         if (mDisconnectCallback != null) {
             mCallbackHandler.post(() -> {
-                synchronized (this) {
-                    if (mDisconnectCallback != null) {
-                        mDisconnectCallback.onDisconnectResult(
-                                DisconnectCallback.DISCONNECT_STATUS_SUCCESS);
-                    }
+                final DisconnectCallback disconnectCallback = mDisconnectCallback;
+                if (disconnectCallback != null) {
+                    disconnectCallback.onDisconnectResult(
+                            DisconnectCallback.DISCONNECT_STATUS_SUCCESS);
                 }
             });
         }
diff --git a/libs/WifiTrackerLib/src/com/android/wifitrackerlib/OsuWifiEntry.java b/libs/WifiTrackerLib/src/com/android/wifitrackerlib/OsuWifiEntry.java
index 67fa79f84..35e582579 100644
--- a/libs/WifiTrackerLib/src/com/android/wifitrackerlib/OsuWifiEntry.java
+++ b/libs/WifiTrackerLib/src/com/android/wifitrackerlib/OsuWifiEntry.java
@@ -205,9 +205,10 @@ class OsuWifiEntry extends WifiEntry {
                     mOsuStatusString =
                             mContext.getString(R.string.wifitrackerlib_osu_connect_failed);
                 }
-                if (mConnectCallback != null) {
-                    mConnectCallback.onConnectResult(CONNECT_STATUS_FAILURE_UNKNOWN);
-                }
+            }
+            final ConnectCallback connectCallback = mConnectCallback;
+            if (connectCallback != null) {
+                connectCallback.onConnectResult(CONNECT_STATUS_FAILURE_UNKNOWN);
             }
             notifyOnUpdated();
         }
@@ -254,12 +255,11 @@ class OsuWifiEntry extends WifiEntry {
             PasspointConfiguration passpointConfig = mWifiManager
                     .getMatchingPasspointConfigsForOsuProviders(Collections.singleton(mOsuProvider))
                     .get(mOsuProvider);
+            final ConnectCallback connectCallback = mConnectCallback;
             if (passpointConfig == null) {
                 // Failed to find the config we just provisioned
-                synchronized (OsuWifiEntry.this) {
-                    if (mConnectCallback != null) {
-                        mConnectCallback.onConnectResult(CONNECT_STATUS_FAILURE_UNKNOWN);
-                    }
+                if (connectCallback != null) {
+                    connectCallback.onConnectResult(CONNECT_STATUS_FAILURE_UNKNOWN);
                 }
                 return;
             }
@@ -287,10 +287,8 @@ class OsuWifiEntry extends WifiEntry {
             }
 
             // Failed to find the network we provisioned for
-            synchronized (OsuWifiEntry.this) {
-                if (mConnectCallback != null) {
-                    mConnectCallback.onConnectResult(CONNECT_STATUS_FAILURE_UNKNOWN);
-                }
+            if (connectCallback != null) {
+                connectCallback.onConnectResult(CONNECT_STATUS_FAILURE_UNKNOWN);
             }
         }
     }
diff --git a/libs/WifiTrackerLib/src/com/android/wifitrackerlib/PasspointWifiEntry.java b/libs/WifiTrackerLib/src/com/android/wifitrackerlib/PasspointWifiEntry.java
index 7c5bbf81e..59994a9e2 100644
--- a/libs/WifiTrackerLib/src/com/android/wifitrackerlib/PasspointWifiEntry.java
+++ b/libs/WifiTrackerLib/src/com/android/wifitrackerlib/PasspointWifiEntry.java
@@ -328,11 +328,9 @@ public class PasspointWifiEntry extends WifiEntry implements WifiEntry.WifiEntry
             mCalledDisconnect = true;
             mDisconnectCallback = callback;
             mCallbackHandler.postDelayed(() -> {
-                synchronized (this) {
-                    if (callback != null && mCalledDisconnect) {
-                        callback.onDisconnectResult(
-                                DisconnectCallback.DISCONNECT_STATUS_FAILURE_UNKNOWN);
-                    }
+                if (callback != null && mCalledDisconnect) {
+                    callback.onDisconnectResult(
+                            DisconnectCallback.DISCONNECT_STATUS_FAILURE_UNKNOWN);
                 }
             }, 10_000 /* delayMillis */);
             mWifiManager.disableEphemeralNetwork(mFqdn);
diff --git a/libs/WifiTrackerLib/src/com/android/wifitrackerlib/WifiEntry.java b/libs/WifiTrackerLib/src/com/android/wifitrackerlib/WifiEntry.java
index 87273dfe3..e194adcb3 100644
--- a/libs/WifiTrackerLib/src/com/android/wifitrackerlib/WifiEntry.java
+++ b/libs/WifiTrackerLib/src/com/android/wifitrackerlib/WifiEntry.java
@@ -701,10 +701,9 @@ public class WifiEntry implements Comparable<WifiEntry> {
     protected void notifyOnUpdated() {
         if (mListener != null) {
             mCallbackHandler.post(() -> {
-                synchronized (this) {
-                    if (mListener != null) {
-                        mListener.onUpdated();
-                    }
+                final WifiEntryCallback listener = mListener;
+                if (listener != null) {
+                    listener.onUpdated();
                 }
             });
         }
@@ -836,8 +835,9 @@ public class WifiEntry implements Comparable<WifiEntry> {
                 if (mCalledConnect) {
                     mCalledConnect = false;
                     mCallbackHandler.post(() -> {
-                        if (mConnectCallback != null) {
-                            mConnectCallback.onConnectResult(
+                        final ConnectCallback connectCallback = mConnectCallback;
+                        if (connectCallback != null) {
+                            connectCallback.onConnectResult(
                                     ConnectCallback.CONNECT_STATUS_SUCCESS);
                         }
                     });
@@ -861,8 +861,9 @@ public class WifiEntry implements Comparable<WifiEntry> {
             if (mCalledDisconnect) {
                 mCalledDisconnect = false;
                 mCallbackHandler.post(() -> {
-                    if (mDisconnectCallback != null) {
-                        mDisconnectCallback.onDisconnectResult(
+                    final DisconnectCallback disconnectCallback = mDisconnectCallback;
+                    if (disconnectCallback != null) {
+                        disconnectCallback.onDisconnectResult(
                                 DisconnectCallback.DISCONNECT_STATUS_SUCCESS);
                     }
                 });
@@ -975,13 +976,12 @@ public class WifiEntry implements Comparable<WifiEntry> {
             }
             // If we aren't connected to the network after 10 seconds, trigger the failure callback
             mCallbackHandler.postDelayed(() -> {
-                synchronized (WifiEntry.this) {
-                    if (mConnectCallback != null && mCalledConnect
-                            && getConnectedState() == CONNECTED_STATE_DISCONNECTED) {
-                        mConnectCallback.onConnectResult(
-                                ConnectCallback.CONNECT_STATUS_FAILURE_UNKNOWN);
-                        mCalledConnect = false;
-                    }
+                final ConnectCallback connectCallback = mConnectCallback;
+                if (connectCallback != null && mCalledConnect
+                        && getConnectedState() == CONNECTED_STATE_DISCONNECTED) {
+                    connectCallback.onConnectResult(
+                            ConnectCallback.CONNECT_STATUS_FAILURE_UNKNOWN);
+                    mCalledConnect = false;
                 }
             }, 10_000 /* delayMillis */);
         }
@@ -989,11 +989,10 @@ public class WifiEntry implements Comparable<WifiEntry> {
         @Override
         public void onFailure(int i) {
             mCallbackHandler.post(() -> {
-                synchronized (WifiEntry.this) {
-                    if (mConnectCallback != null) {
-                        mConnectCallback.onConnectResult(
-                                mConnectCallback.CONNECT_STATUS_FAILURE_UNKNOWN);
-                    }
+                final ConnectCallback connectCallback = mConnectCallback;
+                if (connectCallback != null) {
+                    connectCallback.onConnectResult(
+                            ConnectCallback.CONNECT_STATUS_FAILURE_UNKNOWN);
                 }
             });
         }
@@ -1003,10 +1002,9 @@ public class WifiEntry implements Comparable<WifiEntry> {
         @Override
         public void onSuccess() {
             mCallbackHandler.post(() -> {
-                synchronized (WifiEntry.this) {
-                    if (mForgetCallback != null) {
-                        mForgetCallback.onForgetResult(ForgetCallback.FORGET_STATUS_SUCCESS);
-                    }
+                final ForgetCallback forgetCallback = mForgetCallback;
+                if (forgetCallback != null) {
+                    forgetCallback.onForgetResult(ForgetCallback.FORGET_STATUS_SUCCESS);
                 }
             });
         }
@@ -1014,11 +1012,9 @@ public class WifiEntry implements Comparable<WifiEntry> {
         @Override
         public void onFailure(int i) {
             mCallbackHandler.post(() -> {
-                synchronized (WifiEntry.this) {
-                    if (mForgetCallback != null) {
-                        mForgetCallback.onForgetResult(
-                                ForgetCallback.FORGET_STATUS_FAILURE_UNKNOWN);
-                    }
+                final ForgetCallback forgetCallback = mForgetCallback;
+                if (forgetCallback != null) {
+                    forgetCallback.onForgetResult(ForgetCallback.FORGET_STATUS_FAILURE_UNKNOWN);
                 }
             });
         }