Merge changes Ie826477d,I5125a3ac
* changes:
[CLATJ#26] ClatCoordinator: reword clatd starting failure logging
[CLATJ#25] ClatCoordinator: stop clatd process gracefully
diff --git a/service/jni/com_android_server_connectivity_ClatCoordinator.cpp b/service/jni/com_android_server_connectivity_ClatCoordinator.cpp
index eeaad18..b445462 100644
--- a/service/jni/com_android_server_connectivity_ClatCoordinator.cpp
+++ b/service/jni/com_android_server_connectivity_ClatCoordinator.cpp
@@ -426,6 +426,43 @@
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 @@
}
}
- kill(pid, SIGTERM);
- waitpid(pid, nullptr, 0); // Should we block in JNI?
+ stopClatdProcess(pid);
}
/*
diff --git a/service/src/com/android/server/connectivity/ClatCoordinator.java b/service/src/com/android/server/connectivity/ClatCoordinator.java
index 7616457..c57983b 100644
--- a/service/src/com/android/server/connectivity/ClatCoordinator.java
+++ b/service/src/com/android/server/connectivity/ClatCoordinator.java
@@ -220,7 +220,7 @@
@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 bdcc21b..84e02ce 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 @@
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));