From b758f8497633e0f8a8bf72d571132dc464473266 Mon Sep 17 00:00:00 2001 From: slama Date: Sun, 25 Mar 2018 02:21:15 +0900 Subject: [PATCH] wait for processes by their name - You can now specify the process name instead of process ids and wait for jobs. - Refactor the code of wait command --- src/builtin_wait.cpp | 130 ++++++++++++++++++++++++++++++++++--------- tests/wait.expect | 86 ++++++++++++++++++++++++++-- 2 files changed, 186 insertions(+), 30 deletions(-) diff --git a/src/builtin_wait.cpp b/src/builtin_wait.cpp index 2a1670463..b1d072026 100644 --- a/src/builtin_wait.cpp +++ b/src/builtin_wait.cpp @@ -1,4 +1,5 @@ // Functions for waiting for processes completed. +#include #include #include "builtin.h" @@ -12,6 +13,27 @@ static int retval; +/// Return the job 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_t *get_job_from_pid(pid_t pid) { + job_t *j; + job_iterator_t jobs; + + while (j = jobs.next()) { + if (j->pgid == pid) { + return j; + } + // 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; + } + } + } + return NULL; +} + 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 &wjobs_pid) { +static bool all_specified_jobs_finished(const std::vector &wjobs_pid) { for (auto pid : wjobs_pid) { - if (job_t *j = job_get_from_pid(pid)) { + if (job_t *j = get_job_from_pid(pid)) { // 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 &wjobs_pid) { return true; } -static bool any_specified_jobs_finished(const std::vector &wjobs_pid) { - job_t *j; +static bool any_specified_jobs_finished(const std::vector &wjobs_pid) { for (auto pid : wjobs_pid) { - if ((j = job_get_from_pid(pid))) { + if (job_t *j = get_job_from_pid(pid)) { // If any specified job is completed, return true. if ((j->flags & JOB_CONSTRUCTED) && (job_is_completed(j) || job_is_stopped(j))) { return true; @@ -90,7 +111,7 @@ static bool any_specified_jobs_finished(const std::vector &wjobs_pid) { return false; } -static void wait_for_backgrounds_specified(const std::vector &wjobs_pid, bool any_flag) { +static void wait_for_backgrounds_specified(const std::vector &wjobs_pid, bool any_flag) { while ((!any_flag && !all_specified_jobs_finished(wjobs_pid)) || (any_flag && !any_specified_jobs_finished(wjobs_pid))) { pid_t pid = proc_wait_any(); @@ -101,8 +122,64 @@ static void wait_for_backgrounds_specified(const std::vector &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 &pids) { + 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(pids.begin(), pids.end(), j->pgid) == pids.end()) { + // If pids doesn't already have the pgid, add it. + pids.push_back(j->pgid); + } + 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(pids.begin(), pids.end(), j->pgid) == pids.end()) { + // If pids doesn't already have the pgid, add it. + pids.push_back(j->pgid); + } + 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,28 +216,29 @@ int builtin_wait(parser_t &parser, io_streams_t &streams, wchar_t **argv) { wait_for_backgrounds(any_flag); } else { // jobs specified - std::vector waited_jobs_pid; + std::vector waited_jobs_pid; 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_t *j = get_job_from_pid(pid)) { + waited_jobs_pid.push_back(j->pgid); + } 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_jobs_pid)) { + streams.err.append_format( + _(L"%ls: Could not find child processes with the name '%ls'\n"), cmd, + argv[i]); } } } diff --git a/tests/wait.expect b/tests/wait.expect index 4eb6ec74b..c188f16bf 100644 --- a/tests/wait.expect +++ b/tests/wait.expect @@ -57,7 +57,7 @@ send_line "sleep 3 &; set pid1 \$last_pid; sleep 1 &; set pid2 \$last_pid; sleep expect_prompt send_line "wait -n" expect_prompt "Job 3, 'sleep 1 &' has ended" {} unmatched { puts stderr $error_msg } -send_line "wait -n" +send_line "wait --any" expect_prompt "Job 4, 'sleep 2 &' has ended" {} unmatched { puts stderr $error_msg } send_line "wait -n" expect_prompt "Job 1, 'sleep 3 &' has ended" {} unmatched { puts stderr $error_msg } @@ -77,8 +77,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,7 +98,7 @@ 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 @@ -106,3 +106,81 @@ 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 "Job 4, 'sleep 3 | cat &' has ended" {} unmatched { puts stderr $error_msg } +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 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 }