wait for processes by their name

doc_src/wait.txt

@@ -1,16 +1,34 @@
-\section wait wait - wait for commands to complete
+\section wait wait - wait for jobs to complete
 
 \subsection wait-synopsis Synopsis
 \fish{synopsis}
-wait [-n | --any] PID...
+wait [-n | --any] [PID | PROCESS_NAME] ...
 \endfish
 
 \subsection wait-description Description
 
-`wait` waits for child processes to complete. If a pid is specified, the command waits for that pid. If no pid is specified, the command waits for all background processes.
+`wait` waits for child jobs to complete.
 
-If the `-n` / `--any` flag is provided, the command returns as soon as the first subprocess completes. If it is not provided, it returns after all subprocesses complete.
+- If a pid is specified, the command waits for the job that the process with the pid belongs to.
+- If a process name is specified, the command waits for the jobs that the matched processes belong to.
+- If neither a pid nor a process name is specified, the command waits for all background jobs.
+- If the `-n` / `--any` flag is provided, the command returns as soon as the first job completes. If it is not provided, it returns after all jobs complete.
 
 \subsection wait-example Example
 
-`sleep 10 & ; wait %1` spawns sleep in the background, and then waits until it finishes.
+\fish
+sleep 10 &
+wait $last_pid
+\endfish
+spawns `sleep` in the background, and then waits until it finishes.
+\fish
+for i in (seq 1 5); sleep 10 &; end
+wait
+\endfish
+spawns five jobs in the background, and then waits until all of them finishes.
+\fish
+for i in (seq 1 5); sleep 10 &; end
+hoge &
+wait sleep
+\endfish
+spawns five jobs and `hoge` in the background, and then waits until all `sleep`s finishes, and doesn't wait for `hoge` finishing.

src/builtin_wait.cpp

@@ -1,4 +1,5 @@
 // Functions for waiting for processes completed.
+#include <algorithm>
 #include <vector>
 
 #include "builtin.h"
@@ -12,6 +13,27 @@
 
 static int retval;
 
+/// Return the job id to which the process with pid belongs.
+/// If a specified process has already finished but the job hasn't, parser_t::job_get_from_pid()
+/// doesn't work properly, so use this function in wait command.
+static job_id_t get_job_id_from_pid(pid_t pid) {
+    job_t *j;
+    job_iterator_t jobs;
+
+    while (j = jobs.next()) {
+        if (j->pgid == pid) {
+            return j->job_id;
+        }
+        // Check if the specified pid is a child process of the job.
+        for (const process_ptr_t &p : j->processes) {
+            if (p->pid == pid) {
+                return j->job_id;
+            }
+        }
+    }
+    return 0;
+}
+
 static bool all_jobs_finished() {
     job_iterator_t jobs;
     while (job_t *j = jobs.next()) {
@@ -61,9 +83,9 @@ static void wait_for_backgrounds(bool any_flag) {
     }
 }
 
-static bool all_specified_jobs_finished(const std::vector<int> &wjobs_pid) {
-    for (auto pid : wjobs_pid) {
-        if (job_t *j = job_get_from_pid(pid)) {
+static bool all_specified_jobs_finished(const std::vector<job_id_t> &ids) {
+    for (auto id : ids) {
+        if (job_t *j = job_get(id)) {
             // If any specified job is not completed, return false.
             // If there are stopped jobs, they are ignored.
             if ((j->flags & JOB_CONSTRUCTED) && !job_is_completed(j) && !job_is_stopped(j)) {
@@ -74,10 +96,9 @@ static bool all_specified_jobs_finished(const std::vector<int> &wjobs_pid) {
     return true;
 }
 
-static bool any_specified_jobs_finished(const std::vector<int> &wjobs_pid) {
-    job_t *j;
-    for (auto pid : wjobs_pid) {
-        if ((j = job_get_from_pid(pid))) {
+static bool any_specified_jobs_finished(const std::vector<job_id_t> &ids) {
+    for (auto id : ids) {
+        if (job_t *j = job_get(id)) {
             // If any specified job is completed, return true.
             if ((j->flags & JOB_CONSTRUCTED) && (job_is_completed(j) || job_is_stopped(j))) {
                 return true;
@@ -90,9 +111,9 @@ static bool any_specified_jobs_finished(const std::vector<int> &wjobs_pid) {
     return false;
 }
 
-static void wait_for_backgrounds_specified(const std::vector<int> &wjobs_pid, bool any_flag) {
-    while ((!any_flag && !all_specified_jobs_finished(wjobs_pid)) ||
-           (any_flag && !any_specified_jobs_finished(wjobs_pid))) {
+static void wait_for_backgrounds_specified(const std::vector<job_id_t> &ids, bool any_flag) {
+    while ((!any_flag && !all_specified_jobs_finished(ids)) ||
+           (any_flag && !any_specified_jobs_finished(ids))) {
         pid_t pid = proc_wait_any();
         if (pid == -1 && errno == EINTR) {
             retval = 128 + SIGINT;
@@ -101,8 +122,64 @@ static void wait_for_backgrounds_specified(const std::vector<int> &wjobs_pid, bo
     }
 }
 
+/// Tests if all characters in the wide string are numeric.
+static bool iswnumeric(const wchar_t *n) {
+    for (; *n; n++) {
+        if (*n < L'0' || *n > L'9') {
+            return false;
+        }
+    }
+    return true;
+}
+
+/// See if the process described by \c proc matches the commandline \c cmd.
+static bool match_pid(const wcstring &cmd, const wchar_t *proc) {
+    // Don't wait for itself
+    if (wcscmp(proc, L"wait") == 0) return false;
+
+    // Get the command to match against. We're only interested in the last path component.
+    const wcstring base_cmd = wbasename(cmd);
+    return wcscmp(proc, base_cmd.c_str()) == 0;
+}
+
+/// It should search the job list for something matching the given proc.
+static bool find_job_by_name(const wchar_t *proc, std::vector<job_id_t> &ids) {
+    job_iterator_t jobs;
+    bool found = false;
+
+    while (const job_t *j = jobs.next()) {
+        if (j->command_is_empty()) continue;
+
+        if (match_pid(j->command(), proc)) {
+            if (std::find(ids.begin(), ids.end(), j->job_id) == ids.end()) {
+                // If pids doesn't already have the pgid, add it.
+                ids.push_back(j->job_id);
+            }
+            found = true;
+        }
+
+        // Check if the specified pid is a child process of the job.
+        for (const process_ptr_t &p : j->processes) {
+            if (p->actual_cmd.empty()) continue;
+
+            if (match_pid(p->actual_cmd, proc)) {
+                if (std::find(ids.begin(), ids.end(), j->job_id) == ids.end()) {
+                    // If pids doesn't already have the pgid, add it.
+                    ids.push_back(j->job_id);
+                }
+                found = true;
+            }
+        }
+    }
+
+    return found;
+}
+
+/// The following function is invoked on the main thread, because the job operation is not thread
+/// safe. It waits for child jobs, not for child processes individually.
 int builtin_wait(parser_t &parser, io_streams_t &streams, wchar_t **argv) {
-    job_t *j;
+    ASSERT_IS_MAIN_THREAD();
+
     job_iterator_t jobs;
     const wchar_t *cmd = argv[0];
     int argc = builtin_count_args(argv);
@@ -139,35 +216,36 @@ int builtin_wait(parser_t &parser, io_streams_t &streams, wchar_t **argv) {
         wait_for_backgrounds(any_flag);
     } else {
         // jobs specified
-        std::vector<int> waited_jobs_pid;
+        std::vector<job_id_t> waited_job_ids;
 
         for (int i = w.woptind; i < argc; i++) {
-            int pid = fish_wcstoi(argv[i]);
-            if (errno || pid < 0) {
-                streams.err.append_format(_(L"%ls: '%ls' is not a valid job specifier\n"), cmd,
-                                          argv[i]);
-                continue;
-            }
-            if (job_get_from_pid(pid)) {
-                waited_jobs_pid.push_back(pid);
-            } else {
-                // If a specified process has already finished but the job hasn't,
-                // job_get_from_pid(pid) doesn't work properly, so check the pgid here.
-                while ((j = jobs.next())) {
-                    if (j->pgid == pid) {
-                        waited_jobs_pid.push_back(pid);
-                        break;
-                    }
+            if (iswnumeric(argv[i])) {
+                // argument is pid
+                pid_t pid = fish_wcstoi(argv[i]);
+                if (errno || pid <= 0) {
+                    streams.err.append_format(_(L"%ls: '%ls' is not a valid process id\n"), cmd,
+                                              argv[i]);
+                    continue;
                 }
-                if (!j) {
-                    streams.err.append_format(_(L"%ls: Could not find job '%d'\n"), cmd, pid);
+                if (job_id_t id = get_job_id_from_pid(pid)) {
+                    waited_job_ids.push_back(id);
+                } else {
+                    streams.err.append_format(
+                        _(L"%ls: Could not find a job with process id '%d'\n"), cmd, pid);
+                }
+            } else {
+                // argument is process name
+                if (!find_job_by_name(argv[i], waited_job_ids)) {
+                    streams.err.append_format(
+                        _(L"%ls: Could not find child processes with the name '%ls'\n"), cmd,
+                        argv[i]);
                 }
             }
         }
 
-        if (waited_jobs_pid.empty()) return STATUS_INVALID_ARGS;
+        if (waited_job_ids.empty()) return STATUS_INVALID_ARGS;
 
-        wait_for_backgrounds_specified(waited_jobs_pid, any_flag);
+        wait_for_backgrounds_specified(waited_job_ids, any_flag);
     }
 
     return retval;

tests/wait.expect

@@ -56,10 +56,9 @@ set error_msg "wait with -n option: Fail"
 send_line "sleep 3 &; sleep 1 &; sleep 2 &"
 expect_prompt
 send_line "wait -n"
-expect_prompt "Job 2, 'sleep 1 &' has ended" {} unmatched { puts stderr $error_msg }
-send_line "wait -n"
-expect_prompt "Job 3, 'sleep 2 &' has ended" {} unmatched { puts stderr $error_msg }
-send_line "wait -n"
+expect_prompt "Job 3, 'sleep 1 &' has ended" {} unmatched { puts stderr $error_msg }
+send_line "wait --any"
+expect_prompt "Job 4, 'sleep 2 &' has ended" {} unmatched { puts stderr $error_msg }
 expect_prompt "Job 1, 'sleep 3 &' has ended" {} unmatched { puts stderr $error_msg }
 send_line "jobs"
 expect_prompt "jobs: There are no jobs" {} unmatched { puts stderr $error_msg }
@@ -77,8 +76,8 @@ expect_prompt "Job 1, 'sleep 3 &' has ended" {} unmatched { puts stderr $error_m
 send_line "jobs"
 expect_prompt "jobs: There are no jobs" {} unmatched { puts stderr $error_msg }
 
-# don't wait stopped jobs
-set error_msg "don't wait stopped jobs: Fail"
+# don't wait for stopped jobs
+set error_msg "don't wait for stopped jobs: Fail"
 
 send_line "sleep 3 &"
 expect_prompt
@@ -98,11 +97,99 @@ send_line "jobs"
 expect_prompt "jobs: There are no jobs" {} unmatched { puts stderr $error_msg }
 
 # return immediately when no jobs
-set error_msg "don't wait stopped jobs: Fail"
+set error_msg "return immediately when no jobs: Fail"
 
 send_line "wait"
 expect_prompt
 send_line "wait -n"
 expect_prompt
 send_line "jobs"
 expect_prompt "jobs: There are no jobs" {} unmatched { puts stderr $error_msg }
+
+# wait for jobs by its process name
+set error_msg "wait for jobs by its process name: Fail"
+
+send_line "for i in (seq 1 10); sleep 2 &; end"
+expect_prompt
+send_line "wait sleep"
+expect_prompt
+send_line "jobs"
+expect_prompt "jobs: There are no jobs" {} unmatched { puts stderr $error_msg }
+
+# wait for jobs by its process name with -n option
+set error_msg "wait for jobs by its process name with -n option: Fail"
+
+send_line "for i in (seq 1 3); sleep \$i &; end"
+expect_prompt
+send_line "wait -n sleep"
+expect_prompt
+send_line "jobs | wc -l"
+expect "2" {} timeout { puts stderr $error_msg }
+expect_prompt
+send_line "wait"
+expect_prompt
+send_line "jobs"
+expect_prompt "jobs: There are no jobs" {} unmatched { puts stderr $error_msg }
+
+# complex case
+set error_msg "complex case: Fail"
+
+send_line "for i in (seq 1 10); ls | sleep 2 | cat > /dev/null &; end"
+expect_prompt
+send_line "sleep 3 | cat &"
+expect_prompt
+send_line "sleep 1 &"
+expect_prompt
+send_line "wait \$last_pid sleep"
+expect_prompt
+send_line "jobs"
+expect_prompt "jobs: There are no jobs" {} unmatched { puts stderr $error_msg }
+
+# complex case 2
+set error_msg "complex case 2: Fail"
+
+send_line "for i in (seq 2 4); ls | sleep \$i | cat > /dev/null &; end"
+expect_prompt
+send_line "sleep 3 | cat &"
+expect_prompt
+send_line "sleep 1 &"
+expect_prompt
+send_line "wait -n cat"
+expect_prompt
+send_line "jobs | wc -l"
+expect "3" {} timeout { puts stderr $error_msg }
+expect_prompt
+send_line "wait"
+expect_prompt
+send_line "jobs"
+expect_prompt "jobs: There are no jobs" {} unmatched { puts stderr $error_msg }
+
+# don't wait for itself
+set error_msg "don't wait for itself: Fail"
+
+send_line "wait wait"
+expect_prompt "wait: Could not find child processes with the name 'wait'" {} unmatched { puts stderr $error_msg }
+send_line "wait -n wait"
+expect_prompt "wait: Could not find child processes with the name 'wait'" {} unmatched { puts stderr $error_msg }
+send_line "jobs"
+expect_prompt "jobs: There are no jobs" {} unmatched { puts stderr $error_msg }
+
+# test with fish script
+set error_msg "test with fish script: Fail"
+
+send_line "fish -c 'sleep 2 &; sleep 1 &; wait \$last_pid; jobs | wc -l'"
+expect "1" {} timeout { puts stderr $error_msg }
+expect_prompt
+send_line "fish -c 'sleep 2 &; sleep 3 &; sleep 1 &; wait -n sleep; jobs | wc -l'"
+expect "1" {} timeout { puts stderr $error_msg }
+expect_prompt
+
+# test error messages
+set error_msg "test error messages: Fail"
+
+send_line "wait 0"
+expect_prompt "wait: '0' is not a valid process id" {} unmatched { puts stderr $error_msg }
+send_line "wait 1"
+expect_prompt "wait: Could not find a job with process id '1'" {} unmatched { puts stderr $error_msg }
+send_line "wait hoge"
+expect_prompt "wait: Could not find child processes with the name 'hoge'" {} unmatched { puts stderr $error_msg }