From 4976772f9b5099d814d43e0abeff825d07184f58 Mon Sep 17 00:00:00 2001 From: Lorenzo Colitti Date: Fri, 1 May 2015 00:30:10 +0900 Subject: [PATCH] Fix a race when removing from LegacyTypeTracker. Because LegacyTypeTracker#remove can send broadcasts that cause apps to refresh their view of network state, it needs to be called only after network state has been updated. This requires that callers determine whether the network was the default, and updating state, before calling remove(). While I'm at it, fix maybeLogBroadcast's concept of whether the network it's logging about is/was the default. This has never been correct. Bug: 20613953 Change-Id: Ia175ac454aa4e0a4c4f0151866314ebada681438 --- .../android/server/ConnectivityService.java | 41 +++++++++++-------- 1 file changed, 23 insertions(+), 18 deletions(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 1dc2d7e719..0961ffe1ff 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -463,11 +463,12 @@ public class ConnectivityService extends IConnectivityManager.Stub } } - private void maybeLogBroadcast(NetworkAgentInfo nai, boolean connected, int type) { + private void maybeLogBroadcast(NetworkAgentInfo nai, boolean connected, int type, + boolean isDefaultNetwork) { if (DBG) { log("Sending " + (connected ? "connected" : "disconnected") + " broadcast for type " + type + " " + nai.name() + - " isDefaultNetwork=" + isDefaultNetwork(nai)); + " isDefaultNetwork=" + isDefaultNetwork); } } @@ -487,43 +488,45 @@ public class ConnectivityService extends IConnectivityManager.Stub list.add(nai); // Send a broadcast if this is the first network of its type or if it's the default. - if (list.size() == 1 || isDefaultNetwork(nai)) { - maybeLogBroadcast(nai, true, type); + final boolean isDefaultNetwork = isDefaultNetwork(nai); + if (list.size() == 1 || isDefaultNetwork) { + maybeLogBroadcast(nai, true, type, isDefaultNetwork); sendLegacyNetworkBroadcast(nai, true, type); } } /** Removes the given network from the specified legacy type list. */ - public void remove(int type, NetworkAgentInfo nai) { + public void remove(int type, NetworkAgentInfo nai, boolean wasDefault) { ArrayList list = mTypeLists[type]; if (list == null || list.isEmpty()) { return; } - boolean wasFirstNetwork = list.get(0).equals(nai); + final boolean wasFirstNetwork = list.get(0).equals(nai); if (!list.remove(nai)) { return; } - if (wasFirstNetwork || isDefaultNetwork(nai)) { - maybeLogBroadcast(nai, false, type); + if (wasFirstNetwork || wasDefault) { + maybeLogBroadcast(nai, false, type, wasDefault); sendLegacyNetworkBroadcast(nai, false, type); } if (!list.isEmpty() && wasFirstNetwork) { if (DBG) log("Other network available for type " + type + ", sending connected broadcast"); - maybeLogBroadcast(list.get(0), false, type); - sendLegacyNetworkBroadcast(list.get(0), false, type); + final NetworkAgentInfo replacement = list.get(0); + maybeLogBroadcast(replacement, false, type, isDefaultNetwork(replacement)); + sendLegacyNetworkBroadcast(replacement, false, type); } } /** Removes the given network from all legacy type lists. */ - public void remove(NetworkAgentInfo nai) { - if (VDBG) log("Removing agent " + nai); + public void remove(NetworkAgentInfo nai, boolean wasDefault) { + if (VDBG) log("Removing agent " + nai + " wasDefault=" + wasDefault); for (int type = 0; type < mTypeLists.length; type++) { - remove(type, nai); + remove(type, nai, wasDefault); } } @@ -2036,12 +2039,13 @@ public class ConnectivityService extends IConnectivityManager.Stub loge("Error connecting NetworkAgent"); NetworkAgentInfo nai = mNetworkAgentInfos.remove(msg.replyTo); if (nai != null) { + final boolean wasDefault = isDefaultNetwork(nai); synchronized (mNetworkForNetId) { mNetworkForNetId.remove(nai.network.netId); mNetIdInUse.delete(nai.network.netId); } // Just in case. - mLegacyTypeTracker.remove(nai); + mLegacyTypeTracker.remove(nai, wasDefault); } } } @@ -2071,7 +2075,8 @@ public class ConnectivityService extends IConnectivityManager.Stub nai.networkInfo.setDetailedState(NetworkInfo.DetailedState.DISCONNECTED, null, null); } - if (isDefaultNetwork(nai)) { + final boolean wasDefault = isDefaultNetwork(nai); + if (wasDefault) { mDefaultInetConditionPublished = 0; } notifyIfacesChanged(); @@ -2079,7 +2084,6 @@ public class ConnectivityService extends IConnectivityManager.Stub nai.networkMonitor.sendMessage(NetworkMonitor.CMD_NETWORK_DISCONNECTED); mNetworkAgentInfos.remove(msg.replyTo); updateClat(null, nai.linkProperties, nai); - mLegacyTypeTracker.remove(nai); synchronized (mNetworkForNetId) { mNetworkForNetId.remove(nai.network.netId); mNetIdInUse.delete(nai.network.netId); @@ -2118,6 +2122,7 @@ public class ConnectivityService extends IConnectivityManager.Stub notifyLockdownVpn(nai); requestNetworkTransitionWakelock(nai.name()); } + mLegacyTypeTracker.remove(nai, wasDefault); for (NetworkAgentInfo networkToActivate : toActivate) { unlinger(networkToActivate); rematchNetworkAndRequests(networkToActivate, NascentState.NOT_JUST_VALIDATED, @@ -2307,7 +2312,7 @@ public class ConnectivityService extends IConnectivityManager.Stub } if (doRemove) { - mLegacyTypeTracker.remove(nri.request.legacyType, nai); + mLegacyTypeTracker.remove(nri.request.legacyType, nai, false); } } @@ -4190,7 +4195,7 @@ public class ConnectivityService extends IConnectivityManager.Stub // the new one connected. if (oldDefaultNetwork != null) { mLegacyTypeTracker.remove(oldDefaultNetwork.networkInfo.getType(), - oldDefaultNetwork); + oldDefaultNetwork, true); } mDefaultInetConditionPublished = newNetwork.everValidated ? 100 : 0; mLegacyTypeTracker.add(newNetwork.networkInfo.getType(), newNetwork);