Fix race when starting NetworkMonitor
NetworkMonitor obtained LinkProperties and NetworkCapabilities via
synchronous calls to ConnectivityManager after receiving an asynchronous
notification, which is prone to races: the network could be gone before
the LinkProperties/NetworkCapabilities can be fetched.
Fix the race by passing LinkProperties/NetworkCapabilities directly to
NetworkMonitor in the asynchronous notifications.
Test: atest FrameworksNetTests NetworkStackTests
Test: booted, WiFi works
Bug: 129375892
Change-Id: I200ac7ca6ff79590b11c9be705f650c92fd3cb63
diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java
index 58807de..873bd49 100644
--- a/services/core/java/com/android/server/ConnectivityService.java
+++ b/services/core/java/com/android/server/ConnectivityService.java
@@ -5382,7 +5382,7 @@
mContext, mTrackerHandler, new NetworkMisc(networkMisc), this, mNetd, mNMS,
factorySerialNumber);
// Make sure the network capabilities reflect what the agent info says.
- nai.networkCapabilities = mixInCapabilities(nai, nc);
+ nai.setNetworkCapabilities(mixInCapabilities(nai, nc));
final String extraInfo = networkInfo.getExtraInfo();
final String name = TextUtils.isEmpty(extraInfo)
? nai.networkCapabilities.getSSID() : extraInfo;
@@ -5475,12 +5475,12 @@
// Start or stop DNS64 detection and 464xlat according to network state.
networkAgent.clatd.update();
notifyIfacesChangedForNetworkStats();
+ try {
+ networkAgent.networkMonitor().notifyLinkPropertiesChanged(newLp);
+ } catch (RemoteException e) {
+ e.rethrowFromSystemServer();
+ }
if (networkAgent.everConnected) {
- try {
- networkAgent.networkMonitor().notifyLinkPropertiesChanged();
- } catch (RemoteException e) {
- e.rethrowFromSystemServer();
- }
notifyNetworkCallbacks(networkAgent, ConnectivityManager.CALLBACK_IP_CHANGED);
}
}
@@ -5708,7 +5708,7 @@
final NetworkCapabilities prevNc;
synchronized (nai) {
prevNc = nai.networkCapabilities;
- nai.networkCapabilities = newNc;
+ nai.setNetworkCapabilities(newNc);
}
updateUids(nai, prevNc, newNc);
@@ -5723,11 +5723,6 @@
// If the requestable capabilities have changed or the score changed, we can't have been
// called by rematchNetworkAndRequests, so it's safe to start a rematch.
rematchAllNetworksAndRequests(nai, oldScore);
- try {
- nai.networkMonitor().notifyNetworkCapabilitiesChanged();
- } catch (RemoteException e) {
- e.rethrowFromSystemServer();
- }
notifyNetworkCallbacks(nai, ConnectivityManager.CALLBACK_CAP_CHANGED);
}
@@ -5986,11 +5981,6 @@
}
if (capabilitiesChanged) {
- try {
- nai.networkMonitor().notifyNetworkCapabilitiesChanged();
- } catch (RemoteException e) {
- e.rethrowFromSystemServer();
- }
notifyNetworkCallbacks(nai, ConnectivityManager.CALLBACK_CAP_CHANGED);
}
@@ -6399,7 +6389,8 @@
if (networkAgent.networkMisc.acceptPartialConnectivity) {
networkAgent.networkMonitor().setAcceptPartialConnectivity();
}
- networkAgent.networkMonitor().notifyNetworkConnected();
+ networkAgent.networkMonitor().notifyNetworkConnected(
+ networkAgent.linkProperties, networkAgent.networkCapabilities);
} catch (RemoteException e) {
e.rethrowFromSystemServer();
}
diff --git a/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java b/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java
index 8f2825c..aa05d9a 100644
--- a/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java
+++ b/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java
@@ -29,6 +29,7 @@
import android.os.Handler;
import android.os.INetworkManagementService;
import android.os.Messenger;
+import android.os.RemoteException;
import android.os.SystemClock;
import android.util.Log;
import android.util.SparseArray;
@@ -120,7 +121,8 @@
// This Network object is always valid.
public final Network network;
public LinkProperties linkProperties;
- // This should only be modified via ConnectivityService.updateCapabilities().
+ // This should only be modified by ConnectivityService, via setNetworkCapabilities().
+ // TODO: make this private with a getter.
public NetworkCapabilities networkCapabilities;
public final NetworkMisc networkMisc;
// Indicates if netd has been told to create this Network. From this point on the appropriate
@@ -278,6 +280,25 @@
mNetworkMonitor = networkMonitor;
}
+ /**
+ * Set the NetworkCapabilities on this NetworkAgentInfo. Also attempts to notify NetworkMonitor
+ * of the new capabilities, if NetworkMonitor has been created.
+ *
+ * <p>If {@link NetworkMonitor#notifyNetworkCapabilitiesChanged(NetworkCapabilities)} fails,
+ * the exception is logged but not reported to callers.
+ */
+ public void setNetworkCapabilities(NetworkCapabilities nc) {
+ networkCapabilities = nc;
+ final INetworkMonitor nm = mNetworkMonitor;
+ if (nm != null) {
+ try {
+ nm.notifyNetworkCapabilitiesChanged(nc);
+ } catch (RemoteException e) {
+ Log.e(TAG, "Error notifying NetworkMonitor of updated NetworkCapabilities", e);
+ }
+ }
+ }
+
public ConnectivityService connService() {
return mConnService;
}
diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java
index 6f48da3..c7d46e6 100644
--- a/tests/net/java/com/android/server/ConnectivityServiceTest.java
+++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java
@@ -496,7 +496,7 @@
};
try {
- doAnswer(validateAnswer).when(mNetworkMonitor).notifyNetworkConnected();
+ doAnswer(validateAnswer).when(mNetworkMonitor).notifyNetworkConnected(any(), any());
doAnswer(validateAnswer).when(mNetworkMonitor).forceReevaluation(anyInt());
} catch (RemoteException e) {
fail(e.getMessage());