Process enqueued messages from the NAI synchronously.
As ConnectivityService is ready to accept messages about a network
agent, it should process messages immediately and before messages
from other agents are processed.
This is because of minute ordering issues between the agent process,
ConnectivityService and the network stack. In particular, as the agent
is created, ConnectivityService requests the NetworkMonitor from the
network stack ; while this is happening, the process creating the
agent may have sent a bunch of other messages... including, but not
limited to, disconnects or creation of other agents.
The operative case is the case where a disconnect is in the queue.
agent1.register
→ CS creates the monitor for agent 1
agent1.unregisterAfterReplacement
→ CS enqueues in agent1 pending creation of the monitor
agent2.register
→ CS creates the monitor for agent 2
monitor for agent 1 is created
→ CS posts a message to complete agent 1 creation
monitor for agent 2 is created
→ CS posts a message to complete agent 2 creation
agent 1 creation completes
→ CS re-posts the unregisterAfterReplacement message
agent 2 creation completes
...and this is where things fail, because unregistering agent 1
isn't processed yet. If they use the same interface (which is
a common use case for unregisterAfterReplacement) then this
creation will fail.
To address this, this patch will process all agent1 messages
synchronously as the creation completes. It will solve the case
above, and it passes all tests for early network creation.
However, it is worth noting that this is not actually correct,
because it does not guarantee that all messages are processed
in order. Namely, two agents have separate queues and
therefore if they send interleaved messages while CS is
waiting for the NetworkMonitor to be created, the order of
these messages will be lost. e.g.
agent1.register
agent2.register
agent1.unregisterAfterReplacement
The case above SHOULD fail if the agents have the same
interface, but with this patch it will likely succeed because
the call to agent1.unregisterAfterReplacement will be received
while the two NetworkMonitor are being created, and then
be processed as agent1 finishes registration.
The correct change, therefore, would really enqueue all
messages when *any* agent (of the same process at least)
is being created. However, it is arguable whether it's worth
building that change over this one, and it is even arguable
whether the behavior of this here change might not be
better, insofar as two agent messages are generally
unrelated to each other except for unregistering.
Test: CtsNetTestCases ConnectivityCoverageTests
Change-Id: Idf75e4e96c67a456cf0979a79321603b46e8c99c
diff --git a/service/src/com/android/server/ConnectivityService.java b/service/src/com/android/server/ConnectivityService.java
index 69d83e8..dc4a35b 100644
--- a/service/src/com/android/server/ConnectivityService.java
+++ b/service/src/com/android/server/ConnectivityService.java
@@ -4697,6 +4697,10 @@
// If the network has been destroyed, the only thing that it can do is disconnect.
if (nai.isDestroyed() && !isDisconnectRequest(msg)) {
+ if (DBG) {
+ log("Message " + eventName(msg.what) + " from destroyed agent with netId "
+ + nai.network.netId);
+ }
return;
}
@@ -4705,6 +4709,10 @@
// when registration is complete. It does this by sending all the
// messages in the order received immediately after the
// EVENT_AGENT_REGISTERED message.
+ if (DBG) {
+ log("Message " + eventName(msg.what) + " enqueued for agent with netId "
+ + nai.network.netId);
+ }
return;
}
@@ -9457,6 +9465,7 @@
NetworkInfo networkInfo = nai.networkInfo;
updateNetworkInfo(nai, networkInfo);
updateVpnUids(nai, null, nai.networkCapabilities);
+ nai.processEnqueuedMessages(mTrackerHandler::handleMessage);
}
private class NetworkOfferInfo implements IBinder.DeathRecipient {
diff --git a/service/src/com/android/server/connectivity/NetworkAgentInfo.java b/service/src/com/android/server/connectivity/NetworkAgentInfo.java
index 4540f02..abab6ab 100644
--- a/service/src/com/android/server/connectivity/NetworkAgentInfo.java
+++ b/service/src/com/android/server/connectivity/NetworkAgentInfo.java
@@ -25,8 +25,8 @@
import static android.net.NetworkCapabilities.TRANSPORT_TEST;
import static android.net.NetworkCapabilities.TRANSPORT_WIFI;
import static android.net.NetworkCapabilities.transportNamesOf;
-import static android.system.OsConstants.EIO;
import static android.system.OsConstants.EEXIST;
+import static android.system.OsConstants.EIO;
import static android.system.OsConstants.ENOENT;
import static com.android.net.module.util.FrameworkConnectivityStatsLog.CORE_NETWORKING_TERRIBLE_ERROR_OCCURRED;
@@ -93,6 +93,7 @@
import java.util.Objects;
import java.util.SortedSet;
import java.util.TreeSet;
+import java.util.function.Consumer;
/**
* A bag class used by ConnectivityService for holding a collection of most recent
@@ -717,8 +718,20 @@
}
mHandler.obtainMessage(EVENT_AGENT_REGISTERED, ARG_AGENT_SUCCESS, 0, this).sendToTarget();
+ }
+
+ /**
+ * Pass all enqueued messages to the message processor argument, and clear the queue.
+ *
+ * This is called by ConnectivityService when it is ready to receive messages for this
+ * network agent. The processor may process the messages synchronously or asynchronously
+ * at its option.
+ *
+ * @param messageProcessor a function to process the messages
+ */
+ public void processEnqueuedMessages(final Consumer<Message> messageProcessor) {
for (final Message enqueued : mMessagesPendingRegistration) {
- mHandler.sendMessage(enqueued);
+ messageProcessor.accept(enqueued);
}
mMessagesPendingRegistration.clear();
}