From 151e4d41ed30a49d7652d2d5a331eb14f9db86f0 Mon Sep 17 00:00:00 2001 From: AnotherTest Date: Sun, 12 Jul 2020 18:51:47 +0430 Subject: [PATCH] Shell: Put children in their own process groups and fix job control This commit fixes job control by putting children in their own process group, and proxying TTY signals to active jobs. This also cleans up the code around builtin_disown a bit to use the newer job interfaces. --- Shell/Builtin.cpp | 35 ++++++++++++----------------------- Shell/Job.h | 5 +++++ Shell/Shell.cpp | 26 ++++++++++++++++---------- Shell/Shell.h | 3 +++ Shell/main.cpp | 15 +++++++++++++-- 5 files changed, 49 insertions(+), 35 deletions(-) diff --git a/Shell/Builtin.cpp b/Shell/Builtin.cpp index a2e77c4183..1eace438f0 100644 --- a/Shell/Builtin.cpp +++ b/Shell/Builtin.cpp @@ -372,44 +372,33 @@ int Shell::builtin_disown(int argc, const char** argv) fprintf(stderr, "disown: Invalid job id %s\n", job_id); } - if (job_ids.is_empty()) { - u64 id = 0; - for (auto& job : jobs) - id = max(id, job.value->job_id()); - job_ids.append(id); - } + if (job_ids.is_empty()) + job_ids.append(find_last_job_id()); - Vector keys_of_jobs_to_disown; + Vector jobs_to_disown; for (auto id : job_ids) { - bool found = false; - for (auto& entry : jobs) { - if (entry.value->job_id() == id) { - keys_of_jobs_to_disown.append(entry.key); - found = true; - break; - } - } - if (!found) { + auto job = find_job(id); + if (!job) fprintf(stderr, "disown: job with id %zu not found\n", id); - } + else + jobs_to_disown.append(job); } - if (keys_of_jobs_to_disown.is_empty()) { - if (str_job_ids.is_empty()) { + + if (jobs_to_disown.is_empty()) { + if (str_job_ids.is_empty()) fprintf(stderr, "disown: no current job\n"); - } // An error message has already been printed about the nonexistence of each listed job. return 1; } - for (auto job_index : keys_of_jobs_to_disown) { - auto job = jobs.get(job_index).value(); + for (auto job : jobs_to_disown) { job->deactivate(); if (!job->is_running_in_background()) fprintf(stderr, "disown warning: job %" PRIu64 " is currently not running, 'kill -%d %d' to make it continue\n", job->job_id(), SIGCONT, job->pid()); - jobs.remove(job_index); + jobs.remove(job->pid()); } return 0; diff --git a/Shell/Job.h b/Shell/Job.h index c674136942..239473fd87 100644 --- a/Shell/Job.h +++ b/Shell/Job.h @@ -75,6 +75,11 @@ public: bool should_be_disowned() const { return m_should_be_disowned; } void disown() { m_should_be_disowned = true; } bool is_running_in_background() const { return m_running_in_background; } + void unblock() const + { + if (!m_exited && on_exit) + on_exit(*this); + } Function)> on_exit; Core::ElapsedTimer& timer() { return m_command_timer; } diff --git a/Shell/Shell.cpp b/Shell/Shell.cpp index 2ee2e9f949..0b42ee6222 100644 --- a/Shell/Shell.cpp +++ b/Shell/Shell.cpp @@ -474,6 +474,7 @@ RefPtr Shell::run_command(const AST::Command& command) return nullptr; } if (child == 0) { + setpgid(0, 0); tcsetattr(0, TCSANOW, &default_termios); for (auto& rewiring : rewirings) { #ifdef SH_DEBUG @@ -523,6 +524,8 @@ RefPtr Shell::run_command(const AST::Command& command) jobs.set((u64)child, job); job->on_exit = [](auto job) { + if (!job->exited()) + return; if (job->is_running_in_background()) fprintf(stderr, "Shell: Job %d(%s) exited\n", job->pid(), job->cmd().characters()); job->disown(); @@ -598,6 +601,8 @@ void Shell::restore_stdin() void Shell::block_on_job(RefPtr job) { + TemporaryChange current_job { m_current_job, job.ptr() }; + if (!job) return; @@ -1051,18 +1056,10 @@ void Shell::stop_all_jobs() #ifdef SH_DEBUG dbg() << "Job " << entry.value->pid() << " is not running in background"; #endif - if (killpg(entry.value->pgid(), SIGCONT) < 0) { - perror("killpg(CONT)"); - } + kill_job(entry.value, SIGCONT); } - if (killpg(entry.value->pgid(), SIGHUP) < 0) { - perror("killpg(HUP)"); - } - - if (killpg(entry.value->pgid(), SIGTERM) < 0) { - perror("killpg(TERM)"); - } + kill_job(entry.value, SIGHUP); } usleep(10000); // Wait for a bit before killing the job @@ -1099,6 +1096,15 @@ const Job* Shell::find_job(u64 id) return nullptr; } +void Shell::kill_job(const Job* job, int sig) +{ + if (!job) + return; + + if (killpg(job->pgid(), sig) < 0) + perror("killpg(job)"); +} + void Shell::save_to(JsonObject& object) { Core::Object::save_to(object); diff --git a/Shell/Shell.h b/Shell/Shell.h index 72b500ab40..177a22eab2 100644 --- a/Shell/Shell.h +++ b/Shell/Shell.h @@ -111,6 +111,8 @@ public: u64 find_last_job_id() const; const Job* find_job(u64 id); + const Job* current_job() const { return m_current_job; } + void kill_job(const Job*, int sig); String get_history_path(); void load_history(); @@ -163,6 +165,7 @@ private: void cache_path(); void stop_all_jobs(); + const Job* m_current_job { nullptr }; virtual void custom_event(Core::CustomEvent&) override; diff --git a/Shell/main.cpp b/Shell/main.cpp index b26ae28092..2d041679f5 100644 --- a/Shell/main.cpp +++ b/Shell/main.cpp @@ -62,16 +62,21 @@ int main(int argc, char** argv) Core::EventLoop::register_signal(SIGINT, [](int) { editor->interrupted(); + s_shell->kill_job(s_shell->current_job(), SIGINT); }); Core::EventLoop::register_signal(SIGWINCH, [](int) { editor->resized(); + s_shell->kill_job(s_shell->current_job(), SIGWINCH); }); Core::EventLoop::register_signal(SIGTTIN, [](int) {}); Core::EventLoop::register_signal(SIGTTOU, [](int) {}); Core::EventLoop::register_signal(SIGHUP, [](int) { + for (auto& it : s_shell->jobs) + s_shell->kill_job(it.value.ptr(), SIGHUP); + s_shell->save_history(); }); @@ -102,6 +107,8 @@ int main(int argc, char** argv) job.value->set_has_exit(WEXITSTATUS(wstatus)); } else if (WIFSIGNALED(wstatus) && !WIFSTOPPED(wstatus)) { job.value->set_has_exit(126); + } else if (WIFSTOPPED(wstatus)) { + job.value->unblock(); } } if (job.value->should_be_disowned()) @@ -111,8 +118,12 @@ int main(int argc, char** argv) jobs.remove(key); }); - // Ignore SIGTSTP as the shell should not be suspended with ^Z. - Core::EventLoop::register_signal(SIGTSTP, [](auto) {}); + Core::EventLoop::register_signal(SIGTSTP, [](auto) { + auto job = s_shell->current_job(); + s_shell->kill_job(job, SIGTSTP); + if (job) + job->unblock(); + }); #ifndef __serenity__ sigset_t blocked;