From 31475db1a42bf22056fbe32c3945790259da436b Mon Sep 17 00:00:00 2001 From: Robert Greenwalt Date: Tue, 9 Sep 2014 14:46:37 -0700 Subject: [PATCH] Don't accept score below 0. Network Factories are allowed to go below, but networks need to be constrained. Allowing the network to go below 0 meant that -1 could sometimes leak through and foul the logic. The core of 17361330 will be fixed when we stop sending scores for listens to NetworkFactories, but it exposed this issue too. Summary: 1 - add a network listener. This isn't a request so it's not sent to networks. 2 - alter your score (ethernet sets score to -1 when the link goes down) (16:07:39.782) 3 - a bug in ConnectivityService causes score changes to get sent for all network requests and network listeners causing NetworkFactories to no see 2 entities. This bug will be fixed by a pending change (https://googleplex-android-review.googlesource.com/#/c/540840/). This causes the ethernet NetworkFactory to see two entities, both served by networks of score -1. (16:07:39.989) 4 - disconnect Ethernet - this only sends 0 scores for known requests, not network listeners. Had it been sent for both entities they both would have evaluated that the networkfactory score (-1) was lower than the request score (0) and both released their refcount. (16:08:03.147) 5 - this means the listener is tracked by the EthernetNetworkFactory with a score of -1 while the factory itself has a score of -1 so the network release isn't called. bug:17361330 Change-Id: Ife34ca0f9c233dd3c3df80f6fea580af43afcdeb --- core/java/android/net/NetworkAgent.java | 3 +++ .../core/java/com/android/server/ConnectivityService.java | 5 +++++ 2 files changed, 8 insertions(+) diff --git a/core/java/android/net/NetworkAgent.java b/core/java/android/net/NetworkAgent.java index 8df9916deb..80e5b91ecc 100644 --- a/core/java/android/net/NetworkAgent.java +++ b/core/java/android/net/NetworkAgent.java @@ -223,6 +223,9 @@ public abstract class NetworkAgent extends Handler { * Called by the bearer code when it has a new score for this network. */ public void sendNetworkScore(int score) { + if (score < 0) { + throw new IllegalArgumentException("Score must be >= 0"); + } queueOrSendMessage(EVENT_NETWORK_SCORE_CHANGED, new Integer(score)); } diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 94124652b9..060ca17d63 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -4686,6 +4686,11 @@ public class ConnectivityService extends IConnectivityManager.Stub { private void updateNetworkScore(NetworkAgentInfo nai, int score) { if (DBG) log("updateNetworkScore for " + nai.name() + " to " + score); + if (score < 0) { + loge("updateNetworkScore for " + nai.name() + " got a negative score (" + score + + "). Bumping score to min of 0"); + score = 0; + } nai.currentScore = score;