Made minijail_kill return the same code as minijail_wait

Bug: chromium:1007557
Test: Unit tests pass
Change-Id: I26118292e0880a4ad2d39a0a59b0dd06ff338a52

3 files changed, 50 insertions, 10 deletions

diff --git a/libminijail.c b/libminijail.c
index 250b946d..7cc862d9 100644
--- a/libminijail.c
+++ b/libminijail.c
@@ -3111,12 +3111,13 @@ static int minijail_run_internal(struct minijail *j,
 
 int API minijail_kill(struct minijail *j)
 {
-	int st;
+	if (j->initpid <= 0)
+		return -ECHILD;
+
 	if (kill(j->initpid, SIGTERM))
 		return -errno;
-	if (waitpid(j->initpid, &st, 0) < 0)
-		return -errno;
-	return st;
+
+	return minijail_wait(j);
 }
 
 int API minijail_wait(struct minijail *j)
diff --git a/libminijail.h b/libminijail.h
index 67c515c7..3da845c3 100644
--- a/libminijail.h
+++ b/libminijail.h
@@ -404,19 +404,23 @@ int minijail_run_env_pid_pipes_no_preload(struct minijail *j,
 pid_t minijail_fork(struct minijail *j);
 
 /*
- * Kill the specified minijail. The minijail must have been created with pid
- * namespacing; if it was, all processes inside it are atomically killed.
+ * Send SIGTERM to the process in the minijail and wait for it to terminate.
+ *
+ * Return the same nonnegative exit status as minijail_wait(), or a negative
+ * error code (eg -ESRCH if the process has already been waited for).
+ *
+ * This is most useful if the minijail has been created with PID namespacing
+ * since, in this case, all processes inside it are atomically killed.
  */
 int minijail_kill(struct minijail *j);
 
 /*
  * Wait for the first process spawned in the specified minijail to exit, and
- * return its exit status. A process can only be awaited once.
+ * return its exit status. A process can only be waited once.
  *
  * Return:
- *   A negative error code if the process cannot be awaited for (eg -ECHILD if
- *   no process has been started or if the process has already been awaited
- *   for).
+ *   A negative error code if the process cannot be waited for (eg -ECHILD if no
+ *   process has been started or if the process has already been waited for).
  *   MINIJAIL_ERR_NO_COMMAND if command cannot be found.
  *   MINIJAIL_ERR_NO_ACCESS if command cannot be run.
  *   MINIJAIL_ERR_JAIL if process was killed by SIGSYS.
diff --git a/libminijail_unittest.cc b/libminijail_unittest.cc
index d090d500..afcf527a 100644
--- a/libminijail_unittest.cc
+++ b/libminijail_unittest.cc
@@ -217,6 +217,41 @@ TEST_F(MarshalTest, 0xff) {
   EXPECT_EQ(-EINVAL, minijail_unmarshal(j_, buf_, sizeof(buf_)));
 }
 
+TEST(KillTest, running_process) {
+  const ScopedMinijail j(minijail_new());
+  char* const argv[] = {"sh", "-c", "sleep 1000", nullptr};
+  EXPECT_EQ(minijail_run(j.get(), kShellPath, argv), 0);
+  EXPECT_EQ(minijail_kill(j.get()), 128 + SIGTERM);
+  EXPECT_EQ(minijail_kill(j.get()), -ESRCH);
+}
+
+TEST(KillTest, process_already_awaited) {
+  const ScopedMinijail j(minijail_new());
+  char* const argv[] = {"sh", "-c", "sleep 1; exit 42", nullptr};
+  EXPECT_EQ(minijail_run(j.get(), kShellPath, argv), 0);
+  EXPECT_EQ(minijail_wait(j.get()), 42);
+  EXPECT_EQ(minijail_kill(j.get()), -ESRCH);
+}
+
+TEST(KillTest, process_already_finished_but_not_awaited) {
+  int fds[2];
+  const ScopedMinijail j(minijail_new());
+  char* const argv[] = {"sh", "-c", "exit 42", nullptr};
+  ASSERT_EQ(pipe(fds), 0);
+  EXPECT_EQ(minijail_run(j.get(), kShellPath, argv), 0);
+  ASSERT_EQ(close(fds[1]), 0);
+  // Wait for process to finish.
+  char buf[PIPE_BUF];
+  EXPECT_EQ(read(fds[0], buf, PIPE_BUF), 0);
+  EXPECT_EQ(minijail_kill(j.get()), 42);
+  EXPECT_EQ(minijail_wait(j.get()), -ECHILD);
+}
+
+TEST(KillTest, process_not_started) {
+  const ScopedMinijail j(minijail_new());
+  EXPECT_EQ(minijail_kill(j.get()), -ECHILD);
+}
+
 TEST(WaitTest, return_zero) {
   const ScopedMinijail j(minijail_new());
   char* const argv[] = {"sh", "-c", "exit 0", nullptr};