Simplify the return condition in stop()
Previously, the return condition in stop() will check if the state is STOPPING and the reason is ERROR_INVALID_NETWORK. The condition is too restricted so that if another event is happened after binder died, the exception will be thrown and crash the system. Since calling stop() twice doesn't make sense, so relax the condition of return when the state is STOPPING. Bug: 182586681 Test: atest FrameworksNetTests Change-Id: I2454c1c080d8954dd3785d4ac6e96fc4131fdb47
This commit is contained in:
@@ -373,12 +373,10 @@ public class KeepaliveTracker {
|
|||||||
Log.e(TAG, "Cannot stop unowned keepalive " + mSlot + " on " + mNai.network);
|
Log.e(TAG, "Cannot stop unowned keepalive " + mSlot + " on " + mNai.network);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
// Ignore the case when the network disconnects immediately after stop() has been
|
// To prevent races from re-entrance of stop(), return if the state is already stopping.
|
||||||
// called and the keepalive code is waiting for the response from the modem. This
|
// This might happen if multiple event sources stop keepalive in a short time. Such as
|
||||||
// might happen when the caller listens for a lower-layer network disconnect
|
// network disconnect after user calls stop(), or tear down socket after binder died.
|
||||||
// callback and stop the keepalive at that time. But the stop() races with the
|
if (mStartedState == STOPPING) return;
|
||||||
// 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.
|
// Store the reason of stopping, and report it after the keepalive is fully stopped.
|
||||||
if (mStopReason != ERROR_STOP_REASON_UNINITIALIZED) {
|
if (mStopReason != ERROR_STOP_REASON_UNINITIALIZED) {
|
||||||
|
|||||||
@@ -5450,6 +5450,8 @@ public class ConnectivityServiceTest {
|
|||||||
// the follow-up network disconnection will be processed first.
|
// the follow-up network disconnection will be processed first.
|
||||||
mWiFiNetworkAgent.setKeepaliveResponseDelay(3 * TIMEOUT_MS);
|
mWiFiNetworkAgent.setKeepaliveResponseDelay(3 * TIMEOUT_MS);
|
||||||
ka.stop();
|
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
|
// 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.
|
// flaky since the actual stop call to the service is delegated to executor thread.
|
||||||
|
|||||||
Reference in New Issue
Block a user