Merge changes Ie826477d,I5125a3ac

* changes:
  [CLATJ#26] ClatCoordinator: reword clatd starting failure logging
  [CLATJ#25] ClatCoordinator: stop clatd process gracefully

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);
+    stopClatdProcess(pid);
-    waitpid(pid, nullptr, 0);  // Should we block in JNI?
 }
 
 /*

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);

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));