From 50eef8316cbe035f3bef98b757edb35669a3466e Mon Sep 17 00:00:00 2001 From: Mark Date: Tue, 19 Sep 2023 06:56:14 +0000 Subject: [PATCH] SyncSM8.1 always call transitionTo under processMessage As state transitions always occur after a message has been processed, calling transitionTo only under processMessage makes the flow easier to understand and less error-prone. The risk of this change is very minimal since it is only about handling the error, but tethering will stop regardless. Test: atest TetheringTests Change-Id: I56c6cf6cc989464ee84a8333ac131afc808a3d95 --- Tethering/src/android/net/ip/IpServer.java | 16 ++++++++++++-- .../tethering/util/StateMachineShim.java | 22 ++++++++++++++++++- .../tethering/util/StateMachineShimTest.kt | 7 ++++++ 3 files changed, 42 insertions(+), 3 deletions(-) diff --git a/Tethering/src/android/net/ip/IpServer.java b/Tethering/src/android/net/ip/IpServer.java index 6063526fec..c04d0dcfcd 100644 --- a/Tethering/src/android/net/ip/IpServer.java +++ b/Tethering/src/android/net/ip/IpServer.java @@ -237,6 +237,7 @@ public class IpServer extends StateMachineShim { public static final int CMD_NEW_PREFIX_REQUEST = BASE_IPSERVER + 12; // request from PrivateAddressCoordinator to restart tethering. public static final int CMD_NOTIFY_PREFIX_CONFLICT = BASE_IPSERVER + 13; + public static final int CMD_SERVICE_FAILED_TO_START = BASE_IPSERVER + 14; private final State mInitialState; private final State mLocalHotspotState; @@ -510,7 +511,8 @@ public class IpServer extends StateMachineShim { private void handleError() { mLastError = TETHER_ERROR_DHCPSERVER_ERROR; - transitionTo(mInitialState); + sendMessageAtFrontOfQueueToAsyncSM(CMD_SERVICE_FAILED_TO_START, + TETHER_ERROR_DHCPSERVER_ERROR); } } @@ -1120,7 +1122,14 @@ public class IpServer extends StateMachineShim { startServingInterface(); if (mLastError != TETHER_ERROR_NO_ERROR) { - transitionTo(mInitialState); + // This will transition to InitialState right away, regardless of whether any + // message is already waiting in the StateMachine queue (including maybe some + // message to go to InitialState). InitialState will then process any pending + // message (and generally ignores them). It is difficult to know for sure whether + // this is correct in all cases, but this is equivalent to what IpServer was doing + // in previous versions of the mainline module. + // TODO : remove this after migrating to the Sync StateMachine. + sendMessageAtFrontOfQueueToAsyncSM(CMD_SERVICE_FAILED_TO_START, mLastError); } if (DBG) Log.d(TAG, getStateString(mDesiredInterfaceState) + " serve " + mIfaceName); @@ -1210,6 +1219,9 @@ public class IpServer extends StateMachineShim { mCallback.requestEnableTethering(mInterfaceType, false /* enabled */); transitionTo(mWaitingForRestartState); break; + case CMD_SERVICE_FAILED_TO_START: + mLog.e("start serving fail, error: " + message.arg1); + transitionTo(mInitialState); default: return false; } diff --git a/Tethering/src/com/android/networkstack/tethering/util/StateMachineShim.java b/Tethering/src/com/android/networkstack/tethering/util/StateMachineShim.java index fc432f7439..078a35f0e2 100644 --- a/Tethering/src/com/android/networkstack/tethering/util/StateMachineShim.java +++ b/Tethering/src/com/android/networkstack/tethering/util/StateMachineShim.java @@ -29,7 +29,7 @@ import java.util.List; /** A wrapper to decide whether use synchronous state machine for tethering. */ public class StateMachineShim { // Exactly one of mAsyncSM or mSyncSM is non-null. - private final StateMachine mAsyncSM; + private final AsyncStateMachine mAsyncSM; private final SyncStateMachine mSyncSM; /** @@ -148,6 +148,21 @@ public class StateMachineShim { mAsyncSM.sendMessageDelayed(what, delayMillis); } + /** + * Enqueue a message to the front of the queue. + * Protected, may only be called by instances of async state machine. + * + * Message is ignored if state machine has quit. + */ + protected void sendMessageAtFrontOfQueueToAsyncSM(int what, int arg1) { + if (mSyncSM != null) { + throw new IllegalStateException("sendMessageAtFrontOfQueue can only be used with" + + " async SM"); + } + + mAsyncSM.sendMessageAtFrontOfQueueToAsyncSM(what, arg1); + } + /** * Send self message. * This can only be used with sync state machine, so this will throw if using async state @@ -172,5 +187,10 @@ public class StateMachineShim { public AsyncStateMachine(final String name, final Looper looper) { super(name, looper); } + + /** Enqueue a message to the front of the queue for this state machine. */ + public void sendMessageAtFrontOfQueueToAsyncSM(int what, int arg1) { + sendMessageAtFrontOfQueue(what, arg1); + } } } diff --git a/Tethering/tests/unit/src/com/android/networkstack/tethering/util/StateMachineShimTest.kt b/Tethering/tests/unit/src/com/android/networkstack/tethering/util/StateMachineShimTest.kt index 2c4df76c34..f8e98e34b8 100644 --- a/Tethering/tests/unit/src/com/android/networkstack/tethering/util/StateMachineShimTest.kt +++ b/Tethering/tests/unit/src/com/android/networkstack/tethering/util/StateMachineShimTest.kt @@ -78,6 +78,10 @@ class StateMachineShimTest { shimUsingSyncSM.sendMessageDelayedToAsyncSM(what, 1000 /* delayMillis */) } + assertFailsWith(IllegalStateException::class) { + shimUsingSyncSM.sendMessageAtFrontOfQueueToAsyncSM(what, arg1) + } + shimUsingSyncSM.sendSelfMessageToSyncSM(what, obj) inOrder.verify(mSyncSM).sendSelfMessage(what, 0, 0, obj) @@ -119,6 +123,9 @@ class StateMachineShimTest { shimUsingAsyncSM.sendMessageDelayedToAsyncSM(what, 1000 /* delayMillis */) inOrder.verify(mAsyncSM).sendMessageDelayed(what, 1000) + shimUsingAsyncSM.sendMessageAtFrontOfQueueToAsyncSM(what, arg1) + inOrder.verify(mAsyncSM).sendMessageAtFrontOfQueueToAsyncSM(what, arg1) + assertFailsWith(IllegalStateException::class) { shimUsingAsyncSM.sendSelfMessageToSyncSM(what, obj) }