From 846fb39e8a45a3cf535816a95dab1ced87b399cc Mon Sep 17 00:00:00 2001 From: "Chaitanya Cheemala (xWF)" Date: Tue, 11 Mar 2025 06:05:49 -0700 Subject: [PATCH] Revert "Return the registry in registerNetworkAgent." This reverts commit 70ede3260cf58b02015a71e60823b004ff4e3d83. Reason for revert: Likely culprit for b/402365566 - verifying through ABTD before revert submission. This is part of the standard investigation process, and does not mean your CL will be reverted. Change-Id: If72cc264c5e505348309c6c11dbbfd11adeaf31b --- .../src/android/net/ConnectivityManager.java | 6 +-- .../src/android/net/IConnectivityManager.aidl | 4 +- framework/src/android/net/INetworkAgent.aidl | 2 +- .../android/net/INetworkAgentRegistry.aidl | 2 +- framework/src/android/net/NetworkAgent.java | 48 ++++++------------- .../NetworkAndAgentRegistryParcelable.aidl | 31 ------------ .../android/server/ConnectivityService.java | 13 ++--- .../server/connectivity/NetworkAgentInfo.java | 11 +---- .../src/android/net/cts/NetworkAgentTest.kt | 11 ++--- 9 files changed, 30 insertions(+), 98 deletions(-) delete mode 100644 framework/src/android/net/NetworkAndAgentRegistryParcelable.aidl diff --git a/framework/src/android/net/ConnectivityManager.java b/framework/src/android/net/ConnectivityManager.java index cfa517c0cf..5d99b747b9 100644 --- a/framework/src/android/net/ConnectivityManager.java +++ b/framework/src/android/net/ConnectivityManager.java @@ -3952,8 +3952,7 @@ public class ConnectivityManager { @RequiresPermission(anyOf = { NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK, android.Manifest.permission.NETWORK_FACTORY}) - public NetworkAndAgentRegistryParcelable registerNetworkAgent( - @NonNull INetworkAgent na, @NonNull NetworkInfo ni, + public Network registerNetworkAgent(@NonNull INetworkAgent na, @NonNull NetworkInfo ni, @NonNull LinkProperties lp, @NonNull NetworkCapabilities nc, @NonNull NetworkScore score, @NonNull NetworkAgentConfig config, int providerId) { return registerNetworkAgent(na, ni, lp, nc, null /* localNetworkConfig */, score, config, @@ -3968,8 +3967,7 @@ public class ConnectivityManager { @RequiresPermission(anyOf = { NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK, android.Manifest.permission.NETWORK_FACTORY}) - public NetworkAndAgentRegistryParcelable registerNetworkAgent( - @NonNull INetworkAgent na, @NonNull NetworkInfo ni, + public Network registerNetworkAgent(@NonNull INetworkAgent na, @NonNull NetworkInfo ni, @NonNull LinkProperties lp, @NonNull NetworkCapabilities nc, @Nullable LocalNetworkConfig localNetworkConfig, @NonNull NetworkScore score, @NonNull NetworkAgentConfig config, int providerId) { diff --git a/framework/src/android/net/IConnectivityManager.aidl b/framework/src/android/net/IConnectivityManager.aidl index a270684c40..47b33162d2 100644 --- a/framework/src/android/net/IConnectivityManager.aidl +++ b/framework/src/android/net/IConnectivityManager.aidl @@ -30,7 +30,6 @@ import android.net.LinkProperties; import android.net.LocalNetworkConfig; import android.net.Network; import android.net.NetworkAgentConfig; -import android.net.NetworkAndAgentRegistryParcelable; import android.net.NetworkCapabilities; import android.net.NetworkInfo; import android.net.NetworkRequest; @@ -147,8 +146,7 @@ interface IConnectivityManager void declareNetworkRequestUnfulfillable(in NetworkRequest request); - NetworkAndAgentRegistryParcelable registerNetworkAgent( - in INetworkAgent na, in NetworkInfo ni, in LinkProperties lp, + Network registerNetworkAgent(in INetworkAgent na, in NetworkInfo ni, in LinkProperties lp, in NetworkCapabilities nc, in NetworkScore score, in LocalNetworkConfig localNetworkConfig, in NetworkAgentConfig config, in int factorySerialNumber); diff --git a/framework/src/android/net/INetworkAgent.aidl b/framework/src/android/net/INetworkAgent.aidl index c6beeca869..fa5175c49f 100644 --- a/framework/src/android/net/INetworkAgent.aidl +++ b/framework/src/android/net/INetworkAgent.aidl @@ -26,7 +26,7 @@ import android.net.INetworkAgentRegistry; * @hide */ oneway interface INetworkAgent { - void onRegistered(); + void onRegistered(in INetworkAgentRegistry registry); void onDisconnected(); void onBandwidthUpdateRequested(); void onValidationStatusChanged(int validationStatus, diff --git a/framework/src/android/net/INetworkAgentRegistry.aidl b/framework/src/android/net/INetworkAgentRegistry.aidl index afdd1eec0b..61b27b5f0e 100644 --- a/framework/src/android/net/INetworkAgentRegistry.aidl +++ b/framework/src/android/net/INetworkAgentRegistry.aidl @@ -30,7 +30,7 @@ import android.telephony.data.NrQosSessionAttributes; * Interface for NetworkAgents to send network properties. * @hide */ -interface INetworkAgentRegistry { +oneway interface INetworkAgentRegistry { void sendNetworkCapabilities(in NetworkCapabilities nc); void sendLinkProperties(in LinkProperties lp); // TODO: consider replacing this by "markConnected()" and removing diff --git a/framework/src/android/net/NetworkAgent.java b/framework/src/android/net/NetworkAgent.java index 08f5ecdb7c..cefa1ea072 100644 --- a/framework/src/android/net/NetworkAgent.java +++ b/framework/src/android/net/NetworkAgent.java @@ -98,7 +98,6 @@ public abstract class NetworkAgent { @Nullable private volatile Network mNetwork; - // Null before the agent is registered @Nullable private volatile INetworkAgentRegistry mRegistry; @@ -122,8 +121,6 @@ public abstract class NetworkAgent { private NetworkInfo mNetworkInfo; @NonNull private final Object mRegisterLock = new Object(); - // TODO : move the preconnected queue to the system server and remove this - private boolean mConnected = false; /** * The ID of the {@link NetworkProvider} that created this object, or @@ -609,16 +606,16 @@ public abstract class NetworkAgent { public void handleMessage(Message msg) { switch (msg.what) { case EVENT_AGENT_CONNECTED: { - // TODO : move the pre-connected queue to the system server, and remove - // handling this EVENT_AGENT_CONNECTED message. - synchronized (mPreConnectedQueue) { - if (mConnected) { - log("Received new connection while already connected!"); - } else { - if (VDBG) log("NetworkAgent fully connected"); + if (mRegistry != null) { + log("Received new connection while already connected!"); + } else { + if (VDBG) log("NetworkAgent fully connected"); + synchronized (mPreConnectedQueue) { + final INetworkAgentRegistry registry = (INetworkAgentRegistry) msg.obj; + mRegistry = registry; for (RegistryAction a : mPreConnectedQueue) { try { - a.execute(mRegistry); + a.execute(registry); } catch (RemoteException e) { Log.wtf(LOG_TAG, "Communication error with registry", e); // Fall through @@ -626,7 +623,6 @@ public abstract class NetworkAgent { } mPreConnectedQueue.clear(); } - mConnected = true; } break; } @@ -635,7 +631,7 @@ public abstract class NetworkAgent { // let the client know CS is done with us. onNetworkUnwanted(); synchronized (mPreConnectedQueue) { - mConnected = false; + mRegistry = null; } break; } @@ -762,32 +758,20 @@ public abstract class NetworkAgent { } final ConnectivityManager cm = (ConnectivityManager) mInitialConfiguration.context .getSystemService(Context.CONNECTIVITY_SERVICE); - final NetworkAndAgentRegistryParcelable result; if (mInitialConfiguration.localNetworkConfig == null) { // Call registerNetworkAgent without localNetworkConfig argument to pass // android.net.cts.NetworkAgentTest#testAgentStartsInConnecting in old cts - result = cm.registerNetworkAgent(new NetworkAgentBinder(mHandler), + mNetwork = cm.registerNetworkAgent(new NetworkAgentBinder(mHandler), new NetworkInfo(mInitialConfiguration.info), mInitialConfiguration.properties, mInitialConfiguration.capabilities, mInitialConfiguration.score, mInitialConfiguration.config, providerId); } else { - result = cm.registerNetworkAgent(new NetworkAgentBinder(mHandler), + mNetwork = cm.registerNetworkAgent(new NetworkAgentBinder(mHandler), new NetworkInfo(mInitialConfiguration.info), mInitialConfiguration.properties, mInitialConfiguration.capabilities, mInitialConfiguration.localNetworkConfig, mInitialConfiguration.score, mInitialConfiguration.config, providerId); } - if (null == result && Process.isApplicationUid(Process.myUid())) { - // Let it slide in tests to allow mocking, since NetworkAndAgentRegistryParcelable - // is not public and can't be instantiated by CTS. The danger here is that if - // this happens in production for some reason the code will crash later instead - // of here. If this is a system app, it will still crash as expected. - Log.e(LOG_TAG, "registerNetworkAgent returned null. This agent will not work. " - + "Is ConnectivityManager a mock ?"); - } else { - mNetwork = result.network; - mRegistry = result.registry; - } mInitialConfiguration = null; // All this memory can now be GC'd } return mNetwork; @@ -803,8 +787,8 @@ public abstract class NetworkAgent { } @Override - public void onRegistered() { - mHandler.sendMessage(mHandler.obtainMessage(EVENT_AGENT_CONNECTED)); + public void onRegistered(@NonNull INetworkAgentRegistry registry) { + mHandler.sendMessage(mHandler.obtainMessage(EVENT_AGENT_CONNECTED, registry)); } @Override @@ -929,13 +913,11 @@ public abstract class NetworkAgent { * * @hide */ - public INetworkAgent registerForTest(final Network network, - final INetworkAgentRegistry registry) { + public INetworkAgent registerForTest(final Network network) { log("Registering NetworkAgent for test"); synchronized (mRegisterLock) { mNetwork = network; mInitialConfiguration = null; - mRegistry = registry; } return new NetworkAgentBinder(mHandler); } @@ -976,7 +958,7 @@ public abstract class NetworkAgent { FrameworkConnectivityStatsLog.CORE_NETWORKING_TERRIBLE_ERROR_OCCURRED__ERROR_TYPE__TYPE_MESSAGE_QUEUED_BEFORE_CONNECT ); } - if (mConnected) { + if (mRegistry != null) { try { action.execute(mRegistry); } catch (RemoteException e) { diff --git a/framework/src/android/net/NetworkAndAgentRegistryParcelable.aidl b/framework/src/android/net/NetworkAndAgentRegistryParcelable.aidl deleted file mode 100644 index 8c01bbcda1..0000000000 --- a/framework/src/android/net/NetworkAndAgentRegistryParcelable.aidl +++ /dev/null @@ -1,31 +0,0 @@ -/* - * Copyright (C) 2025 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package android.net; - -import android.net.INetworkAgentRegistry; -import android.net.Network; - -/** - * A pair of Network and NetworkAgentRegistry. - * - * {@hide} - */ -@JavaDerive(toString=true) -parcelable NetworkAndAgentRegistryParcelable { - Network network; - INetworkAgentRegistry registry; -} diff --git a/service/src/com/android/server/ConnectivityService.java b/service/src/com/android/server/ConnectivityService.java index 6453d765b6..b2e49e7f50 100644 --- a/service/src/com/android/server/ConnectivityService.java +++ b/service/src/com/android/server/ConnectivityService.java @@ -228,7 +228,6 @@ import android.net.NattSocketKeepalive; import android.net.Network; import android.net.NetworkAgent; import android.net.NetworkAgentConfig; -import android.net.NetworkAndAgentRegistryParcelable; import android.net.NetworkCapabilities; import android.net.NetworkInfo; import android.net.NetworkInfo.DetailedState; @@ -9317,7 +9316,7 @@ public class ConnectivityService extends IConnectivityManager.Stub { * @param providerId the ID of the provider owning this NetworkAgent. * @return the network created for this agent. */ - public NetworkAndAgentRegistryParcelable registerNetworkAgent(INetworkAgent na, + public Network registerNetworkAgent(INetworkAgent na, NetworkInfo networkInfo, LinkProperties linkProperties, NetworkCapabilities networkCapabilities, @@ -9360,8 +9359,7 @@ public class ConnectivityService extends IConnectivityManager.Stub { } } - private NetworkAndAgentRegistryParcelable registerNetworkAgentInternal( - INetworkAgent na, NetworkInfo networkInfo, + private Network registerNetworkAgentInternal(INetworkAgent na, NetworkInfo networkInfo, LinkProperties linkProperties, NetworkCapabilities networkCapabilities, NetworkScore currentScore, NetworkAgentConfig networkAgentConfig, @Nullable LocalNetworkConfig localNetworkConfig, int providerId, @@ -9393,11 +9391,8 @@ public class ConnectivityService extends IConnectivityManager.Stub { // NetworkAgentInfo registration will finish when the NetworkMonitor is created. // If the network disconnects or sends any other event before that, messages are deferred by // NetworkAgent until nai.connect(), which will be called when finalizing the - // registration. TODO : have NetworkAgentInfo defer them instead. - final NetworkAndAgentRegistryParcelable result = new NetworkAndAgentRegistryParcelable(); - result.network = nai.network; - result.registry = nai.getRegistry(); - return result; + // registration. + return nai.network; } private void handleRegisterNetworkAgent(NetworkAgentInfo nai, INetworkMonitor networkMonitor) { diff --git a/service/src/com/android/server/connectivity/NetworkAgentInfo.java b/service/src/com/android/server/connectivity/NetworkAgentInfo.java index d6bd659946..e762a8e866 100644 --- a/service/src/com/android/server/connectivity/NetworkAgentInfo.java +++ b/service/src/com/android/server/connectivity/NetworkAgentInfo.java @@ -639,7 +639,6 @@ public class NetworkAgentInfo implements NetworkRanker.Scoreable { private final ConnectivityService.Dependencies mConnServiceDeps; private final Context mContext; private final Handler mHandler; - private final NetworkAgentMessageHandler mRegistry; private final QosCallbackTracker mQosCallbackTracker; private final INetd mNetd; @@ -674,7 +673,6 @@ public class NetworkAgentInfo implements NetworkRanker.Scoreable { mNetd = netd; mContext = context; mHandler = handler; - mRegistry = new NetworkAgentMessageHandler(mHandler); this.factorySerialNumber = factorySerialNumber; this.creatorUid = creatorUid; mLingerDurationMs = lingerDurationMs; @@ -703,7 +701,7 @@ public class NetworkAgentInfo implements NetworkRanker.Scoreable { public void notifyRegistered() { try { networkAgent.asBinder().linkToDeath(mDeathMonitor, 0); - networkAgent.onRegistered(); + networkAgent.onRegistered(new NetworkAgentMessageHandler(mHandler)); } catch (RemoteException e) { Log.e(TAG, "Error registering NetworkAgent", e); maybeUnlinkDeathMonitor(); @@ -1119,13 +1117,6 @@ public class NetworkAgentInfo implements NetworkRanker.Scoreable { return mNetworkMonitor; } - /** - * Get the registry in this NetworkAgentInfo. - */ - public INetworkAgentRegistry getRegistry() { - return mRegistry; - } - // Functions for manipulating the requests satisfied by this network. // // These functions must only called on ConnectivityService's main thread. diff --git a/tests/cts/net/src/android/net/cts/NetworkAgentTest.kt b/tests/cts/net/src/android/net/cts/NetworkAgentTest.kt index 7031bfbc80..8fcc703c0a 100644 --- a/tests/cts/net/src/android/net/cts/NetworkAgentTest.kt +++ b/tests/cts/net/src/android/net/cts/NetworkAgentTest.kt @@ -154,10 +154,10 @@ import java.nio.ByteBuffer import java.security.MessageDigest import java.time.Duration import java.util.Arrays +import java.util.Random import java.util.UUID import java.util.concurrent.Executors import kotlin.collections.ArrayList -import kotlin.random.Random import kotlin.test.assertEquals import kotlin.test.assertFailsWith import kotlin.test.assertFalse @@ -266,7 +266,7 @@ class NetworkAgentTest { private class FakeConnectivityService { val mockRegistry = mock(INetworkAgentRegistry::class.java) private var agentField: INetworkAgent? = null - val registry: INetworkAgentRegistry = object : INetworkAgentRegistry.Stub(), + private val registry = object : INetworkAgentRegistry.Stub(), INetworkAgentRegistry by mockRegistry { // asBinder has implementations in both INetworkAgentRegistry.Stub and mockRegistry, so // it needs to be disambiguated. Just fail the test as it should be unused here. @@ -283,7 +283,7 @@ class NetworkAgentTest { fun connect(agent: INetworkAgent) { this.agentField = agent - agent.onRegistered() + agent.onRegistered(registry) } fun disconnect() = agent.onDisconnected() @@ -412,8 +412,7 @@ class NetworkAgentTest { } private fun createNetworkAgentWithFakeCS() = createNetworkAgent().also { - val binder = it.registerForTest(Network(FAKE_NET_ID), mFakeConnectivityService.registry) - mFakeConnectivityService.connect(binder) + mFakeConnectivityService.connect(it.registerForTest(Network(FAKE_NET_ID))) } private fun TestableNetworkAgent.expectPostConnectionCallbacks( @@ -1615,7 +1614,7 @@ class NetworkAgentTest { val s = Os.socket(AF_INET6, SOCK_DGRAM, 0) net.bindSocket(s) val content = ByteArray(16) - Random.nextBytes(content) + Random().nextBytes(content) Os.sendto(s, ByteBuffer.wrap(content), 0, REMOTE_ADDRESS, 7 /* port */) val match = reader.poll(DEFAULT_TIMEOUT_MS) { val udpStart = IPV6_HEADER_LEN + UDP_HEADER_LEN