From 1ff914a543bd8374dab148e5ee00fb9eb4e6f023 Mon Sep 17 00:00:00 2001 From: Hungming Chen Date: Wed, 26 Jan 2022 16:06:15 +0800 Subject: [PATCH 1/2] [CLATJ#25] ClatCoordinator: stop clatd process gracefully Send SIGTERM with timeout first, if fail, SIGKILL. Bug: 212345928 Test: connect/disconnect to IPv6 only network and ping 8.8.8.8 repeat 3 times Change-Id: I5125a3ac7e8ab6d4e8ae7fc7134b8ac8e42ce5dd --- ...id_server_connectivity_ClatCoordinator.cpp | 40 ++++++++++++++++++- 1 file changed, 38 insertions(+), 2 deletions(-) diff --git a/service/jni/com_android_server_connectivity_ClatCoordinator.cpp b/service/jni/com_android_server_connectivity_ClatCoordinator.cpp index eeaad186b3..b445462a9a 100644 --- a/service/jni/com_android_server_connectivity_ClatCoordinator.cpp +++ b/service/jni/com_android_server_connectivity_ClatCoordinator.cpp @@ -426,6 +426,43 @@ static jint com_android_server_connectivity_ClatCoordinator_startClatd( return pid; } +// Stop clatd process. SIGTERM with timeout first, if fail, SIGKILL. +// See stopProcess() in system/netd/server/NetdConstants.cpp. +// TODO: have a function stopProcess(int pid, const char *name) in common location and call it. +static constexpr int WAITPID_ATTEMPTS = 50; +static constexpr int WAITPID_RETRY_INTERVAL_US = 100000; + +static void stopClatdProcess(int pid) { + int err = kill(pid, SIGTERM); + if (err) { + err = errno; + } + if (err == ESRCH) { + ALOGE("clatd child process %d unexpectedly disappeared", pid); + return; + } + if (err) { + ALOGE("Error killing clatd child process %d: %s", pid, strerror(err)); + } + int status = 0; + int ret = 0; + for (int count = 0; ret == 0 && count < WAITPID_ATTEMPTS; count++) { + usleep(WAITPID_RETRY_INTERVAL_US); + ret = waitpid(pid, &status, WNOHANG); + } + if (ret == 0) { + ALOGE("Failed to SIGTERM clatd pid=%d, try SIGKILL", pid); + // TODO: fix that kill failed or waitpid doesn't return. + kill(pid, SIGKILL); + ret = waitpid(pid, &status, 0); + } + if (ret == -1) { + ALOGE("Error waiting for clatd child process %d: %s", pid, strerror(errno)); + } else { + ALOGD("clatd process %d terminated status=%d", pid, status); + } +} + static void com_android_server_connectivity_ClatCoordinator_stopClatd(JNIEnv* env, jobject clazz, jstring iface, jstring pfx96, jstring v4, jstring v6, @@ -448,8 +485,7 @@ static void com_android_server_connectivity_ClatCoordinator_stopClatd(JNIEnv* en } } - kill(pid, SIGTERM); - waitpid(pid, nullptr, 0); // Should we block in JNI? + stopClatdProcess(pid); } /* From 6fb21144b80e4c75c4afadec47df320ae362f18c Mon Sep 17 00:00:00 2001 From: Hungming Chen Date: Wed, 26 Jan 2022 17:12:45 +0800 Subject: [PATCH 2/2] [CLATJ#26] ClatCoordinator: reword clatd starting failure logging Bug: 212345928 Test: build Change-Id: Ie826477d27db26cc49d113ec8bfacaaa3006e26e --- .../src/com/android/server/connectivity/ClatCoordinator.java | 2 +- .../com/android/server/connectivity/ClatCoordinatorTest.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/service/src/com/android/server/connectivity/ClatCoordinator.java b/service/src/com/android/server/connectivity/ClatCoordinator.java index 761645792f..c57983b055 100644 --- a/service/src/com/android/server/connectivity/ClatCoordinator.java +++ b/service/src/com/android/server/connectivity/ClatCoordinator.java @@ -220,7 +220,7 @@ public class ClatCoordinator { @NonNull final IpPrefix nat64Prefix) throws IOException { if (mIface != null || mPid != INVALID_PID) { - throw new IOException("Clatd has started on " + mIface + " (pid " + mPid + ")"); + throw new IOException("Clatd is already running on " + mIface + " (pid " + mPid + ")"); } if (nat64Prefix.getPrefixLength() != 96) { throw new IOException("Prefix must be 96 bits long: " + nat64Prefix); diff --git a/tests/unit/java/com/android/server/connectivity/ClatCoordinatorTest.java b/tests/unit/java/com/android/server/connectivity/ClatCoordinatorTest.java index bdcc21b98e..84e02ce3f4 100644 --- a/tests/unit/java/com/android/server/connectivity/ClatCoordinatorTest.java +++ b/tests/unit/java/com/android/server/connectivity/ClatCoordinatorTest.java @@ -340,7 +340,7 @@ public class ClatCoordinatorTest { inOrder.verifyNoMoreInteractions(); // [2] Start clatd again failed. - assertThrows("java.io.IOException: Clatd has started on test0 (pid 10483)", + assertThrows("java.io.IOException: Clatd is already running on test0 (pid 10483)", IOException.class, () -> coordinator.clatStart(BASE_IFACE, NETID, NAT64_IP_PREFIX));