diff --git a/framework/src/android/net/ConnectivityManager.java b/framework/src/android/net/ConnectivityManager.java index 4224da98dd..17389b4529 100644 --- a/framework/src/android/net/ConnectivityManager.java +++ b/framework/src/android/net/ConnectivityManager.java @@ -2279,23 +2279,12 @@ public class ConnectivityManager { private final ISocketKeepaliveCallback mCallback; private final ExecutorService mExecutor; - private volatile Integer mSlot; - @UnsupportedAppUsage(maxTargetSdk = Build.VERSION_CODES.R, trackingBug = 170729553) public void stop() { try { mExecutor.execute(() -> { try { - if (mSlot != null) { - // TODO : this is incorrect, because in the presence of auto on/off - // keepalive the slot associated with this keepalive can have - // changed. Also, this actually causes another problem where some other - // app might stop your keepalive if it just knows the network and - // the slot and goes through the trouble of grabbing the aidl object. - // This code should use the callback to identify what keepalive to - // stop instead. - mService.stopKeepalive(mNetwork, mSlot); - } + mService.stopKeepalive(mCallback); } catch (RemoteException e) { Log.e(TAG, "Error stopping packet keepalive: ", e); throw e.rethrowFromSystemServer(); @@ -2313,11 +2302,10 @@ public class ConnectivityManager { mExecutor = Executors.newSingleThreadExecutor(); mCallback = new ISocketKeepaliveCallback.Stub() { @Override - public void onStarted(int slot) { + public void onStarted() { final long token = Binder.clearCallingIdentity(); try { mExecutor.execute(() -> { - mSlot = slot; callback.onStarted(); }); } finally { @@ -2330,7 +2318,6 @@ public class ConnectivityManager { final long token = Binder.clearCallingIdentity(); try { mExecutor.execute(() -> { - mSlot = null; callback.onStopped(); }); } finally { @@ -2344,7 +2331,6 @@ public class ConnectivityManager { final long token = Binder.clearCallingIdentity(); try { mExecutor.execute(() -> { - mSlot = null; callback.onError(error); }); } finally { diff --git a/framework/src/android/net/IConnectivityManager.aidl b/framework/src/android/net/IConnectivityManager.aidl index 7db231e838..acbc31ef43 100644 --- a/framework/src/android/net/IConnectivityManager.aidl +++ b/framework/src/android/net/IConnectivityManager.aidl @@ -193,7 +193,7 @@ interface IConnectivityManager void startTcpKeepalive(in Network network, in ParcelFileDescriptor pfd, int intervalSeconds, in ISocketKeepaliveCallback cb); - void stopKeepalive(in Network network, int slot); + void stopKeepalive(in ISocketKeepaliveCallback cb); String getCaptivePortalServerUrl(); diff --git a/framework/src/android/net/ISocketKeepaliveCallback.aidl b/framework/src/android/net/ISocketKeepaliveCallback.aidl index 020fbcacbf..1240e37b94 100644 --- a/framework/src/android/net/ISocketKeepaliveCallback.aidl +++ b/framework/src/android/net/ISocketKeepaliveCallback.aidl @@ -24,7 +24,7 @@ package android.net; oneway interface ISocketKeepaliveCallback { /** The keepalive was successfully started. */ - void onStarted(int slot); + void onStarted(); /** The keepalive was successfully stopped. */ void onStopped(); /** The keepalive was stopped because of an error. */ diff --git a/framework/src/android/net/NattSocketKeepalive.java b/framework/src/android/net/NattSocketKeepalive.java index 4d45e7034e..77137f444a 100644 --- a/framework/src/android/net/NattSocketKeepalive.java +++ b/framework/src/android/net/NattSocketKeepalive.java @@ -91,9 +91,7 @@ public final class NattSocketKeepalive extends SocketKeepalive { protected void stopImpl() { mExecutor.execute(() -> { try { - if (mSlot != null) { - mService.stopKeepalive(mNetwork, mSlot); - } + mService.stopKeepalive(mCallback); } catch (RemoteException e) { Log.e(TAG, "Error stopping socket keepalive: ", e); throw e.rethrowFromSystemServer(); diff --git a/framework/src/android/net/NetworkAgent.java b/framework/src/android/net/NetworkAgent.java index 3ec00d9f03..8fe20de223 100644 --- a/framework/src/android/net/NetworkAgent.java +++ b/framework/src/android/net/NetworkAgent.java @@ -291,7 +291,9 @@ public abstract class NetworkAgent { /** * Requests that the specified keepalive packet be stopped. * - * arg1 = hardware slot number of the keepalive to stop. + * arg1 = unused + * arg2 = error code (SUCCESS) + * obj = callback to identify the keepalive * * Also used internally by ConnectivityService / KeepaliveTracker, with different semantics. * @hide diff --git a/framework/src/android/net/SocketKeepalive.java b/framework/src/android/net/SocketKeepalive.java index 90e5e9b34f..2911ce7afb 100644 --- a/framework/src/android/net/SocketKeepalive.java +++ b/framework/src/android/net/SocketKeepalive.java @@ -21,7 +21,6 @@ import static android.annotation.SystemApi.Client.PRIVILEGED_APPS; import android.annotation.IntDef; import android.annotation.IntRange; import android.annotation.NonNull; -import android.annotation.Nullable; import android.annotation.SystemApi; import android.os.Binder; import android.os.ParcelFileDescriptor; @@ -249,9 +248,6 @@ public abstract class SocketKeepalive implements AutoCloseable { @NonNull protected final Executor mExecutor; /** @hide */ @NonNull protected final ISocketKeepaliveCallback mCallback; - // TODO: remove slot since mCallback could be used to identify which keepalive to stop. - /** @hide */ - @Nullable protected Integer mSlot; /** @hide */ public SocketKeepalive(@NonNull IConnectivityManager service, @NonNull Network network, @@ -263,11 +259,10 @@ public abstract class SocketKeepalive implements AutoCloseable { mExecutor = executor; mCallback = new ISocketKeepaliveCallback.Stub() { @Override - public void onStarted(int slot) { + public void onStarted() { final long token = Binder.clearCallingIdentity(); try { mExecutor.execute(() -> { - mSlot = slot; callback.onStarted(); }); } finally { @@ -280,7 +275,6 @@ public abstract class SocketKeepalive implements AutoCloseable { final long token = Binder.clearCallingIdentity(); try { executor.execute(() -> { - mSlot = null; callback.onStopped(); }); } finally { @@ -293,7 +287,6 @@ public abstract class SocketKeepalive implements AutoCloseable { final long token = Binder.clearCallingIdentity(); try { executor.execute(() -> { - mSlot = null; callback.onError(error); }); } finally { @@ -306,7 +299,6 @@ public abstract class SocketKeepalive implements AutoCloseable { final long token = Binder.clearCallingIdentity(); try { executor.execute(() -> { - mSlot = null; callback.onDataReceived(); }); } finally { diff --git a/framework/src/android/net/TcpSocketKeepalive.java b/framework/src/android/net/TcpSocketKeepalive.java index 51d805e9aa..cda5830485 100644 --- a/framework/src/android/net/TcpSocketKeepalive.java +++ b/framework/src/android/net/TcpSocketKeepalive.java @@ -69,9 +69,7 @@ public final class TcpSocketKeepalive extends SocketKeepalive { protected void stopImpl() { mExecutor.execute(() -> { try { - if (mSlot != null) { - mService.stopKeepalive(mNetwork, mSlot); - } + mService.stopKeepalive(mCallback); } catch (RemoteException e) { Log.e(TAG, "Error stopping packet keepalive: ", e); throw e.rethrowFromSystemServer(); diff --git a/service/src/com/android/server/ConnectivityService.java b/service/src/com/android/server/ConnectivityService.java index f1c68cbf77..61c4fb119d 100755 --- a/service/src/com/android/server/ConnectivityService.java +++ b/service/src/com/android/server/ConnectivityService.java @@ -5577,14 +5577,14 @@ public class ConnectivityService extends IConnectivityManager.Stub } // Sent by KeepaliveTracker to process an app request on the state machine thread. case NetworkAgent.CMD_STOP_SOCKET_KEEPALIVE: { - NetworkAgentInfo nai = getNetworkAgentInfoForNetwork((Network) msg.obj); - if (nai == null) { - Log.e(TAG, "Attempt to stop keepalive on nonexistent network"); + final AutomaticOnOffKeepalive ki = mKeepaliveTracker.getKeepaliveForBinder( + (IBinder) msg.obj); + if (ki == null) { + Log.e(TAG, "Attempt to stop an already stopped keepalive"); return; } - int slot = msg.arg1; - int reason = msg.arg2; - mKeepaliveTracker.handleStopKeepalive(nai, slot, reason); + final int reason = msg.arg2; + mKeepaliveTracker.handleStopKeepalive(ki, reason); break; } case EVENT_REPORT_NETWORK_CONNECTIVITY: { @@ -9865,9 +9865,10 @@ public class ConnectivityService extends IConnectivityManager.Stub } @Override - public void stopKeepalive(Network network, int slot) { + public void stopKeepalive(@NonNull final ISocketKeepaliveCallback cb) { mHandler.sendMessage(mHandler.obtainMessage( - NetworkAgent.CMD_STOP_SOCKET_KEEPALIVE, slot, SocketKeepalive.SUCCESS, network)); + NetworkAgent.CMD_STOP_SOCKET_KEEPALIVE, 0, SocketKeepalive.SUCCESS, + Objects.requireNonNull(cb).asBinder())); } @Override diff --git a/service/src/com/android/server/connectivity/AutomaticOnOffKeepaliveTracker.java b/service/src/com/android/server/connectivity/AutomaticOnOffKeepaliveTracker.java index 18e2dd8e7b..9dcbac8133 100644 --- a/service/src/com/android/server/connectivity/AutomaticOnOffKeepaliveTracker.java +++ b/service/src/com/android/server/connectivity/AutomaticOnOffKeepaliveTracker.java @@ -329,30 +329,6 @@ public class AutomaticOnOffKeepaliveTracker { handleResumeKeepalive(newKi); } - // TODO : this method should be removed ; the keepalives should always be indexed by callback - private int findAutomaticOnOffKeepaliveIndex(@NonNull Network network, int slot) { - ensureRunningOnHandlerThread(); - - int index = 0; - for (AutomaticOnOffKeepalive ki : mAutomaticOnOffKeepalives) { - if (ki.match(network, slot)) { - return index; - } - index++; - } - return -1; - } - - // TODO : this method should be removed ; the keepalives should always be indexed by callback - @Nullable - private AutomaticOnOffKeepalive findAutomaticOnOffKeepalive(@NonNull Network network, - int slot) { - ensureRunningOnHandlerThread(); - - final int index = findAutomaticOnOffKeepaliveIndex(network, slot); - return (index >= 0) ? mAutomaticOnOffKeepalives.get(index) : null; - } - /** * Find the AutomaticOnOffKeepalive associated with a given callback. * @return the keepalive associated with this callback, or null if none @@ -415,17 +391,12 @@ public class AutomaticOnOffKeepaliveTracker { /** * Handle stop keepalives on the specific network with given slot. */ - public void handleStopKeepalive(@NonNull NetworkAgentInfo nai, int slot, int reason) { - final AutomaticOnOffKeepalive autoKi = findAutomaticOnOffKeepalive(nai.network, slot); - if (null == autoKi) { - Log.e(TAG, "Attempt to stop nonexistent keepalive " + slot + " on " + nai); - return; - } - + public void handleStopKeepalive(@NonNull final AutomaticOnOffKeepalive autoKi, int reason) { // Stop the keepalive unless it was suspended. This includes the case where it's managed // but enabled, and the case where it's always on. if (autoKi.mAutomaticOnOffState != STATE_SUSPENDED) { - mKeepaliveTracker.handleStopKeepalive(nai, slot, reason); + final KeepaliveTracker.KeepaliveInfo ki = autoKi.mKi; + mKeepaliveTracker.handleStopKeepalive(ki.getNai(), ki.getSlot(), reason); } cleanupAutoOnOffKeepalive(autoKi); diff --git a/service/src/com/android/server/connectivity/KeepaliveTracker.java b/service/src/com/android/server/connectivity/KeepaliveTracker.java index 7cb613b551..a512b7cc79 100644 --- a/service/src/com/android/server/connectivity/KeepaliveTracker.java +++ b/service/src/com/android/server/connectivity/KeepaliveTracker.java @@ -589,9 +589,9 @@ public class KeepaliveTracker { Log.d(TAG, "Started keepalive " + slot + " on " + nai.toShortString()); ki.mStartedState = KeepaliveInfo.STARTED; try { - ki.mCallback.onStarted(slot); + ki.mCallback.onStarted(); } catch (RemoteException e) { - Log.w(TAG, "Discarded onStarted(" + slot + ") callback"); + Log.w(TAG, "Discarded onStarted callback"); } } else { Log.d(TAG, "Failed to start keepalive " + slot + " on " + nai.toShortString()