drgreen/android_frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix for NetworkStats/Telephony deadlock

Browse Source 
Call into NetworkStatsService to update uid foreground
state without the NPMS lock held. Both calls are from
the handler thread, so no sequencing issues.

Bug: 123274986
Bug: 74007921
Test: atest CtsHostsideNetworkTests
Change-Id: I9e8449e5a75db616e646f55c930ff82982fc9083
This commit is contained in:
Amith Yamasani
2019-02-19 09:57:32 -08:00
parent dd07ae579c
commit d78542bb52
1 changed files with 21 additions and 8 deletions
Download Patch File Download Diff File Expand all files Collapse all files

29
services/core/java/com/android/server/net/NetworkPolicyManagerService.java
View File

@ -3515,10 +3515,10 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub {
/** /**
* Process state of UID changed; if needed, will trigger * Process state of UID changed; if needed, will trigger
* {@link #updateRulesForDataUsageRestrictionsUL(int)} and * {@link #updateRulesForDataUsageRestrictionsUL(int)} and
* {@link #updateRulesForPowerRestrictionsUL(int)} * {@link #updateRulesForPowerRestrictionsUL(int)}. Returns true if the state was updated.
*/ */
@GuardedBy("mUidRulesFirstLock") @GuardedBy("mUidRulesFirstLock")
private void updateUidStateUL(int uid, int uidState) { private boolean updateUidStateUL(int uid, int uidState) {
Trace.traceBegin(Trace.TRACE_TAG_NETWORK, "updateUidStateUL"); Trace.traceBegin(Trace.TRACE_TAG_NETWORK, "updateUidStateUL");
try { try {
final int oldUidState = mUidState.get(uid, ActivityManager.PROCESS_STATE_CACHED_EMPTY); final int oldUidState = mUidState.get(uid, ActivityManager.PROCESS_STATE_CACHED_EMPTY);
@ -3537,15 +3537,16 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub {
} }
updateRulesForPowerRestrictionsUL(uid); updateRulesForPowerRestrictionsUL(uid);
} }
updateNetworkStats(uid, isUidStateForeground(uidState)); return true;
} }
} finally { } finally {
Trace.traceEnd(Trace.TRACE_TAG_NETWORK); Trace.traceEnd(Trace.TRACE_TAG_NETWORK);
} }
return false;
} }
@GuardedBy("mUidRulesFirstLock") @GuardedBy("mUidRulesFirstLock")
private void removeUidStateUL(int uid) { private boolean removeUidStateUL(int uid) {
final int index = mUidState.indexOfKey(uid); final int index = mUidState.indexOfKey(uid);
if (index >= 0) { if (index >= 0) {
final int oldUidState = mUidState.valueAt(index); final int oldUidState = mUidState.valueAt(index);
@ -3560,9 +3561,10 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub {
updateRuleForRestrictPowerUL(uid); updateRuleForRestrictPowerUL(uid);
} }
updateRulesForPowerRestrictionsUL(uid); updateRulesForPowerRestrictionsUL(uid);
updateNetworkStats(uid, false); return true;
} }
} }
return false;
} }
// adjust stats accounting based on foreground status // adjust stats accounting based on foreground status
@ -4552,21 +4554,26 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub {
} }
} }
} }
}; };
void handleUidChanged(int uid, int procState, long procStateSeq) { void handleUidChanged(int uid, int procState, long procStateSeq) {
Trace.traceBegin(Trace.TRACE_TAG_NETWORK, "onUidStateChanged"); Trace.traceBegin(Trace.TRACE_TAG_NETWORK, "onUidStateChanged");
try { try {
boolean updated;
synchronized (mUidRulesFirstLock) { synchronized (mUidRulesFirstLock) {
// We received a uid state change callback, add it to the history so that it // We received a uid state change callback, add it to the history so that it
// will be useful for debugging. // will be useful for debugging.
mLogger.uidStateChanged(uid, procState, procStateSeq); mLogger.uidStateChanged(uid, procState, procStateSeq);
// Now update the network policy rules as per the updated uid state. // Now update the network policy rules as per the updated uid state.
updateUidStateUL(uid, procState); updated = updateUidStateUL(uid, procState);
// Updating the network rules is done, so notify AMS about this. // Updating the network rules is done, so notify AMS about this.
mActivityManagerInternal.notifyNetworkPolicyRulesUpdated(uid, procStateSeq); mActivityManagerInternal.notifyNetworkPolicyRulesUpdated(uid, procStateSeq);
} }
// Do this without the lock held. handleUidChanged() and handleUidGone() are
// called from the handler, so there's no multi-threading issue.
if (updated) {
updateNetworkStats(uid, isUidStateForeground(procState));
}
} finally { } finally {
Trace.traceEnd(Trace.TRACE_TAG_NETWORK); Trace.traceEnd(Trace.TRACE_TAG_NETWORK);
} }
@ -4575,8 +4582,14 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub {
void handleUidGone(int uid) { void handleUidGone(int uid) {
Trace.traceBegin(Trace.TRACE_TAG_NETWORK, "onUidGone"); Trace.traceBegin(Trace.TRACE_TAG_NETWORK, "onUidGone");
try { try {
boolean updated;
synchronized (mUidRulesFirstLock) { synchronized (mUidRulesFirstLock) {
removeUidStateUL(uid); updated = removeUidStateUL(uid);
}
// Do this without the lock held. handleUidChanged() and handleUidGone() are
// called from the handler, so there's no multi-threading issue.
if (updated) {
updateNetworkStats(uid, false);
} }
} finally { } finally {
Trace.traceEnd(Trace.TRACE_TAG_NETWORK); Trace.traceEnd(Trace.TRACE_TAG_NETWORK);