diff --git a/service/src/com/android/server/connectivity/KeepaliveTracker.java b/service/src/com/android/server/connectivity/KeepaliveTracker.java index acf39f05a5..ee1538adf8 100644 --- a/service/src/com/android/server/connectivity/KeepaliveTracker.java +++ b/service/src/com/android/server/connectivity/KeepaliveTracker.java @@ -373,12 +373,10 @@ 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; + // To prevent races from re-entrance of stop(), return if the state is already stopping. + // This might happen if multiple event sources stop keepalive in a short time. Such as + // network disconnect after user calls stop(), or tear down socket after binder died. + if (mStartedState == STOPPING) return; // Store the reason of stopping, and report it after the keepalive is fully stopped. if (mStopReason != ERROR_STOP_REASON_UNINITIALIZED) { diff --git a/tests/unit/java/com/android/server/ConnectivityServiceTest.java b/tests/unit/java/com/android/server/ConnectivityServiceTest.java index 53c46958e4..9601bb1be3 100644 --- a/tests/unit/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/unit/java/com/android/server/ConnectivityServiceTest.java @@ -5450,6 +5450,8 @@ public class ConnectivityServiceTest { // the follow-up network disconnection will be processed first. mWiFiNetworkAgent.setKeepaliveResponseDelay(3 * TIMEOUT_MS); ka.stop(); + // Call stop() twice shouldn't result in crash, b/182586681. + 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.