From 42aba3e38ad5bd69541d357aafd0fd774e91926e Mon Sep 17 00:00:00 2001 From: Paul Jensen Date: Fri, 19 Sep 2014 11:14:12 -0400 Subject: [PATCH] Remove needless locking of mRulesLock that caused deadlocks. The locks were added in c006f1 when underlying functions weren't performing locking. In 21062e7 the underlying functions were changed to perform locking but the higher level locking wasn't removed. The higher level locking can now cause deadlocks with the new NetworkAgentInfo locking. This change removes the needless higher level locking. Now all mRulesLock locking only guards simple accesses to the appropriate two data strucures so there is no chance of a deadlock. I verified that all accesses to the appropriate two data structures are guarded by mRulesLock locking. bug:17569997 Change-Id: Id9f4e3d19d6895876925ae32f12460db30359368 --- .../android/server/ConnectivityService.java | 29 +++++++++---------- 1 file changed, 13 insertions(+), 16 deletions(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 55d8c09da0..ef9bf51aee 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -979,12 +979,10 @@ public class ConnectivityService extends IConnectivityManager.Stub { enforceAccessPermission(); final int uid = Binder.getCallingUid(); final ArrayList result = Lists.newArrayList(); - synchronized (mRulesLock) { - for (int networkType = 0; networkType <= ConnectivityManager.MAX_NETWORK_TYPE; - networkType++) { - if (getNetworkInfoForType(networkType) != null) { - result.add(getFilteredNetworkInfo(networkType, uid)); - } + for (int networkType = 0; networkType <= ConnectivityManager.MAX_NETWORK_TYPE; + networkType++) { + if (getNetworkInfoForType(networkType) != null) { + result.add(getFilteredNetworkInfo(networkType, uid)); } } return result.toArray(new NetworkInfo[result.size()]); @@ -1078,15 +1076,13 @@ public class ConnectivityService extends IConnectivityManager.Stub { enforceAccessPermission(); final int uid = Binder.getCallingUid(); final ArrayList result = Lists.newArrayList(); - synchronized (mRulesLock) { - for (int networkType = 0; networkType <= ConnectivityManager.MAX_NETWORK_TYPE; - networkType++) { - if (getNetworkInfoForType(networkType) != null) { - final NetworkInfo info = getFilteredNetworkInfo(networkType, uid); - final LinkProperties lp = getLinkPropertiesForTypeInternal(networkType); - final NetworkCapabilities netcap = getNetworkCapabilitiesForType(networkType); - result.add(new NetworkState(info, lp, netcap)); - } + for (int networkType = 0; networkType <= ConnectivityManager.MAX_NETWORK_TYPE; + networkType++) { + if (getNetworkInfoForType(networkType) != null) { + final NetworkInfo info = getFilteredNetworkInfo(networkType, uid); + final LinkProperties lp = getLinkPropertiesForTypeInternal(networkType); + final NetworkCapabilities netcap = getNetworkCapabilitiesForType(networkType); + result.add(new NetworkState(info, lp, netcap)); } } return result.toArray(new NetworkState[result.size()]); @@ -4125,8 +4121,9 @@ public class ConnectivityService extends IConnectivityManager.Stub { if (networkCapabilities.hasCapability(NetworkCapabilities.NET_CAPABILITY_NOT_METERED) == false) { final int uidRules; + final int uid = Binder.getCallingUid(); synchronized(mRulesLock) { - uidRules = mUidRules.get(Binder.getCallingUid(), RULE_ALLOW_ALL); + uidRules = mUidRules.get(uid, RULE_ALLOW_ALL); } if ((uidRules & RULE_REJECT_METERED) != 0) { // we could silently fail or we can filter the available nets to only give