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

@@ -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()) {
+    if (job_ids.is_empty())
-        u64 id = 0;
+        job_ids.append(find_last_job_id());
-        for (auto& job : jobs)
-            id = max(id, job.value->job_id());
-        job_ids.append(id);
-    }
 
-    Vector<size_t> keys_of_jobs_to_disown;
+    Vector<const Job*> jobs_to_disown;
 
     for (auto id : job_ids) {
-        bool found = false;
+        auto job = find_job(id);
-        for (auto& entry : jobs) {
+        if (!job)
-            if (entry.value->job_id() == id) {
-                keys_of_jobs_to_disown.append(entry.key);
-                found = true;
-                break;
-            }
-        }
-        if (!found) {
             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 (jobs_to_disown.is_empty()) {
-        if (str_job_ids.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;

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<void(RefPtr<Job>)> on_exit;
 
     Core::ElapsedTimer& timer() { return m_command_timer; }

Shell/Shell.cpp

@@ -474,6 +474,7 @@ RefPtr<Job> 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<Job> 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> job)
 {
+    TemporaryChange<const Job*> 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) {
+                kill_job(entry.value, SIGCONT);
-                    perror("killpg(CONT)");
-                }
             }
 
-            if (killpg(entry.value->pgid(), SIGHUP) < 0) {
+            kill_job(entry.value, SIGHUP);
-                perror("killpg(HUP)");
-            }
-
-            if (killpg(entry.value->pgid(), SIGTERM) < 0) {
-                perror("killpg(TERM)");
-            }
         }
 
         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);

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;
 

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;