From 0d22aded3dcbbf8e28ea1b65ba30c404720874d4 Mon Sep 17 00:00:00 2001 From: Kangping Dong Date: Wed, 20 Mar 2024 23:48:23 +0800 Subject: [PATCH] [Thread] fix ot-daemon state consistency issues 1. the OtDaemonState in OtDaemonCallbackProxy class is not cleared when ot-daemon dies 2. register ot-daemon state callback within the IOtDaemon#initialize API to avoid missing state changes between initialize() and registerStateCallback() Bug: 330441708 Change-Id: Ice58566582b40a6d25627cf01dec93da61ad3596 --- .../ThreadNetworkControllerService.java | 9 +++++--- .../net/thread/ThreadIntegrationTest.java | 7 +++++-- .../net/thread/utils/OtDaemonController.java | 6 ++++++ .../ThreadNetworkControllerServiceTest.java | 21 +++++++++++++++++++ 4 files changed, 38 insertions(+), 5 deletions(-) diff --git a/thread/service/java/com/android/server/thread/ThreadNetworkControllerService.java b/thread/service/java/com/android/server/thread/ThreadNetworkControllerService.java index 964aee21eb..3ef564bdb4 100644 --- a/thread/service/java/com/android/server/thread/ThreadNetworkControllerService.java +++ b/thread/service/java/com/android/server/thread/ThreadNetworkControllerService.java @@ -343,8 +343,8 @@ final class ThreadNetworkControllerService extends IThreadNetworkController.Stub mTunIfController.getTunFd(), isEnabled(), mNsdPublisher, - getMeshcopTxtAttributes(mResources.get())); - otDaemon.registerStateCallback(mOtDaemonCallbackProxy, -1); + getMeshcopTxtAttributes(mResources.get()), + mOtDaemonCallbackProxy); otDaemon.asBinder().linkToDeath(() -> mHandler.post(this::onOtDaemonDied), 0); mOtDaemon = otDaemon; return mOtDaemon; @@ -1329,8 +1329,11 @@ final class ThreadNetworkControllerService extends IThreadNetworkController.Stub return; } + final int deviceRole = mState.deviceRole; + mState = null; + // If this device is already STOPPED or DETACHED, do nothing - if (!ThreadNetworkController.isAttached(mState.deviceRole)) { + if (!ThreadNetworkController.isAttached(deviceRole)) { return; } diff --git a/thread/tests/integration/src/android/net/thread/ThreadIntegrationTest.java b/thread/tests/integration/src/android/net/thread/ThreadIntegrationTest.java index 4a006cf729..bfded1d5dd 100644 --- a/thread/tests/integration/src/android/net/thread/ThreadIntegrationTest.java +++ b/thread/tests/integration/src/android/net/thread/ThreadIntegrationTest.java @@ -97,13 +97,16 @@ public class ThreadIntegrationTest { } @Test - public void otDaemonRestart_JoinedNetworkAndStopped_autoRejoined() throws Exception { + public void otDaemonRestart_JoinedNetworkAndStopped_autoRejoinedAndTunIfStateConsistent() + throws Exception { mController.joinAndWait(DEFAULT_DATASET); runShellCommand("stop ot-daemon"); mController.waitForRole(DEVICE_ROLE_DETACHED, CALLBACK_TIMEOUT); mController.waitForRole(DEVICE_ROLE_LEADER, RESTART_JOIN_TIMEOUT); + assertThat(mOtCtl.isInterfaceUp()).isTrue(); + assertThat(runShellCommand("ifconfig thread-wpan")).contains("UP POINTOPOINT RUNNING"); } @Test @@ -120,8 +123,8 @@ public class ThreadIntegrationTest { mController.joinAndWait(DEFAULT_DATASET); mOtCtl.factoryReset(); - String ifconfig = runShellCommand("ifconfig thread-wpan"); + String ifconfig = runShellCommand("ifconfig thread-wpan"); assertThat(ifconfig).doesNotContain("inet6 addr"); } diff --git a/thread/tests/integration/src/android/net/thread/utils/OtDaemonController.java b/thread/tests/integration/src/android/net/thread/utils/OtDaemonController.java index 4a06fe8676..7c7f9d671c 100644 --- a/thread/tests/integration/src/android/net/thread/utils/OtDaemonController.java +++ b/thread/tests/integration/src/android/net/thread/utils/OtDaemonController.java @@ -62,6 +62,12 @@ public final class OtDaemonController { .toList(); } + /** Returns {@code true} if the Thread interface is up. */ + public boolean isInterfaceUp() { + String output = executeCommand("ifconfig"); + return output.contains("up"); + } + public String executeCommand(String cmd) { return SystemUtil.runShellCommand(OT_CTL + " " + cmd); } diff --git a/thread/tests/unit/src/com/android/server/thread/ThreadNetworkControllerServiceTest.java b/thread/tests/unit/src/com/android/server/thread/ThreadNetworkControllerServiceTest.java index 151ed5bd47..0c7d0864c7 100644 --- a/thread/tests/unit/src/com/android/server/thread/ThreadNetworkControllerServiceTest.java +++ b/thread/tests/unit/src/com/android/server/thread/ThreadNetworkControllerServiceTest.java @@ -78,7 +78,9 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.ArgumentCaptor; import org.mockito.Captor; +import org.mockito.InOrder; import org.mockito.Mock; +import org.mockito.Mockito; import org.mockito.MockitoAnnotations; import java.util.concurrent.CompletableFuture; @@ -489,4 +491,23 @@ public final class ThreadNetworkControllerServiceTest { assertThat(mFakeOtDaemon.isInitialized()).isTrue(); verify(mockJoinReceiver, times(1)).onSuccess(); } + + @Test + public void onOtDaemonDied_joinedNetwork_interfaceStateBackToUp() throws Exception { + mService.initialize(); + final IOperationReceiver mockReceiver = mock(IOperationReceiver.class); + mService.join(DEFAULT_ACTIVE_DATASET, mockReceiver); + mTestLooper.dispatchAll(); + mTestLooper.moveTimeForward(FakeOtDaemon.JOIN_DELAY.toMillis() + 100); + mTestLooper.dispatchAll(); + + Mockito.reset(mMockInfraIfController); + mFakeOtDaemon.terminate(); + mTestLooper.dispatchAll(); + + verify(mMockTunIfController, times(1)).onOtDaemonDied(); + InOrder inOrder = Mockito.inOrder(mMockTunIfController); + inOrder.verify(mMockTunIfController, times(1)).setInterfaceUp(false); + inOrder.verify(mMockTunIfController, times(1)).setInterfaceUp(true); + } }