Merge "Skip stop if keepalive is already in stopping state" am: cb79f645d4

Original change: https://android-review.googlesource.com/c/platform/frameworks/base/+/1424190

Change-Id: I44f945f1804938819506dab56063d2706b27fcfd

3 files changed, 47 insertions, 5 deletions

diff --git a/services/core/java/com/android/server/connectivity/KeepaliveTracker.java b/services/core/java/com/android/server/connectivity/KeepaliveTracker.java
index 1f0066a43538..01fa9e755bd6 100644
--- a/services/core/java/com/android/server/connectivity/KeepaliveTracker.java
+++ b/services/core/java/com/android/server/connectivity/KeepaliveTracker.java
@@ -367,6 +367,13 @@ public class KeepaliveTracker {
                     Log.e(TAG, "Cannot stop unowned keepalive " + mSlot + " on " + mNai.network);
                 }
             }
+            // Ignore the case when the network disconnects immediately after stop() has been
+            // called and the keepalive code is waiting for the response from the modem. This
+            // might happen when the caller listens for a lower-layer network disconnect
+            // callback and stop the keepalive at that time. But the stop() races with the
+            // stop() generated in ConnectivityService network disconnection code path.
+            if (mStartedState == STOPPING && reason == ERROR_INVALID_NETWORK) return;
+
             // Store the reason of stopping, and report it after the keepalive is fully stopped.
             if (mStopReason != ERROR_STOP_REASON_UNINITIALIZED) {
                 throw new IllegalStateException("Unexpected stop reason: " + mStopReason);
@@ -380,9 +387,6 @@ public class KeepaliveTracker {
                     // e.g. invalid parameter.
                     cleanupStoppedKeepalive(mNai, mSlot);
                     break;
-                case STOPPING:
-                    // Keepalive is already in stopping state, ignore.
-                    return;
                 default:
                     mStartedState = STOPPING;
                     switch (mType) {
diff --git a/tests/net/integration/util/com/android/server/NetworkAgentWrapper.java b/tests/net/integration/util/com/android/server/NetworkAgentWrapper.java
index 9f0b41fa0cdf..c895420157d7 100644
--- a/tests/net/integration/util/com/android/server/NetworkAgentWrapper.java
+++ b/tests/net/integration/util/com/android/server/NetworkAgentWrapper.java
@@ -67,6 +67,9 @@ public class NetworkAgentWrapper implements TestableNetworkCallback.HasNetwork {
     private NetworkAgent mNetworkAgent;
     private int mStartKeepaliveError = SocketKeepalive.ERROR_UNSUPPORTED;
     private int mStopKeepaliveError = SocketKeepalive.NO_KEEPALIVE;
+    // Controls how test network agent is going to wait before responding to keepalive
+    // start/stop. Useful when simulate KeepaliveTracker is waiting for response from modem.
+    private long mKeepaliveResponseDelay = 0L;
     private Integer mExpectedKeepaliveSlot = null;
 
     public NetworkAgentWrapper(int transport, LinkProperties linkProperties, Context context)
@@ -134,12 +137,17 @@ public class NetworkAgentWrapper implements TestableNetworkCallback.HasNetwork {
             if (mWrapper.mExpectedKeepaliveSlot != null) {
                 assertEquals((int) mWrapper.mExpectedKeepaliveSlot, slot);
             }
-            onSocketKeepaliveEvent(slot, mWrapper.mStartKeepaliveError);
+            mWrapper.mHandlerThread.getThreadHandler().postDelayed(
+                    () -> onSocketKeepaliveEvent(slot, mWrapper.mStartKeepaliveError),
+                    mWrapper.mKeepaliveResponseDelay);
         }
 
         @Override
         public void stopSocketKeepalive(Message msg) {
-            onSocketKeepaliveEvent(msg.arg1, mWrapper.mStopKeepaliveError);
+            final int slot = msg.arg1;
+            mWrapper.mHandlerThread.getThreadHandler().postDelayed(
+                    () -> onSocketKeepaliveEvent(slot, mWrapper.mStopKeepaliveError),
+                    mWrapper.mKeepaliveResponseDelay);
         }
 
         @Override
@@ -248,6 +256,10 @@ public class NetworkAgentWrapper implements TestableNetworkCallback.HasNetwork {
         mStopKeepaliveError = reason;
     }
 
+    public void setKeepaliveResponseDelay(long delay) {
+        mKeepaliveResponseDelay = delay;
+    }
+
     public void setExpectedKeepaliveSlot(Integer slot) {
         mExpectedKeepaliveSlot = slot;
     }
diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java
index 57c356dae9f8..862e552811f0 100644
--- a/tests/net/java/com/android/server/ConnectivityServiceTest.java
+++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java
@@ -4292,6 +4292,32 @@ public class ConnectivityServiceTest {
         myNet = connectKeepaliveNetwork(lp);
         mWiFiNetworkAgent.setStartKeepaliveEvent(SocketKeepalive.SUCCESS);
 
+        // Check that a stop followed by network disconnects does not result in crash.
+        try (SocketKeepalive ka = mCm.createSocketKeepalive(
+                myNet, testSocket, myIPv4, dstIPv4, executor, callback)) {
+            ka.start(validKaInterval);
+            callback.expectStarted();
+            // Delay the response of keepalive events in networkAgent long enough to make sure
+            // the follow-up network disconnection will be processed first.
+            mWiFiNetworkAgent.setKeepaliveResponseDelay(3 * TIMEOUT_MS);
+            ka.stop();
+
+            // Make sure the stop has been processed. Wait for executor idle is needed to prevent
+            // flaky since the actual stop call to the service is delegated to executor thread.
+            waitForIdleSerialExecutor(executor, TIMEOUT_MS);
+            waitForIdle();
+
+            mWiFiNetworkAgent.disconnect();
+            mWiFiNetworkAgent.expectDisconnected();
+            callback.expectStopped();
+            callback.assertNoCallback();
+        }
+
+        // Reconnect.
+        waitForIdle();
+        myNet = connectKeepaliveNetwork(lp);
+        mWiFiNetworkAgent.setStartKeepaliveEvent(SocketKeepalive.SUCCESS);
+
         // Check that keepalive slots start from 1 and increment. The first one gets slot 1.
         mWiFiNetworkAgent.setExpectedKeepaliveSlot(1);
         int srcPort2 = 0;