Shell: Fix event loop processing and backgrounding in subshells

Author: https://github.com/alimpfard
Commit: https://github.com/SerenityOS/serenity/commit/54b453be578
Pull-request: https://github.com/SerenityOS/serenity/pull/3372
Reviewed-by: https://github.com/tomuta

Shell/Shell.cpp

@@ -510,19 +510,6 @@ RefPtr<Job> Shell::run_command(const AST::Command& command)
             return nullptr;
     }
 
-    if (command.should_immediately_execute_next) {
-        ASSERT(command.argv.is_empty());
-
-        SavedFileDescriptors fds { rewirings };
-        if (apply_rewirings() == IterationDecision::Break)
-            return nullptr;
-
-        for (auto& next_in_chain : command.next_chain)
-            run_tail(next_in_chain, 0);
-
-        return nullptr;
-    }
-
     int retval = 0;
     if (run_builtin(command, rewirings, retval)) {
         for (auto& next_in_chain : command.next_chain)
@@ -554,8 +541,14 @@ RefPtr<Job> Shell::run_command(const AST::Command& command)
     if (child == 0) {
         close(sync_pipe[1]);
 
+        if (!m_is_subshell)
             tcsetattr(0, TCSANOW, &default_termios);
 
+        m_is_subshell = true;
+        m_pid = getpid();
+        Core::EventLoop::notify_forked(Core::EventLoop::ForkEvent::Child);
+        jobs.clear();
+
         if (apply_rewirings() == IterationDecision::Break)
             return nullptr;
 
@@ -573,6 +566,19 @@ RefPtr<Job> Shell::run_command(const AST::Command& command)
 
         close(sync_pipe[0]);
 
+        if (command.should_immediately_execute_next) {
+            ASSERT(command.argv.is_empty());
+
+            Core::EventLoop mainloop;
+
+            setup_signals();
+
+            for (auto& next_in_chain : command.next_chain)
+                run_tail(next_in_chain, 0);
+
+            _exit(last_return_code);
+        }
+
         int rc = execvp(argv[0], const_cast<char* const*>(argv.data()));
         if (rc < 0) {
             if (errno == ENOENT) {
@@ -612,10 +618,10 @@ RefPtr<Job> Shell::run_command(const AST::Command& command)
     }
 
     pid_t pgid = is_first ? child : (command.pipeline ? command.pipeline->pgid : child);
+    if (!m_is_subshell && command.should_wait) {
         if (setpgid(child, pgid) < 0)
             perror("setpgid");
 
-    if (command.should_wait) {
         tcsetpgrp(STDOUT_FILENO, pgid);
         tcsetpgrp(STDIN_FILENO, pgid);
     }
@@ -634,7 +640,12 @@ RefPtr<Job> Shell::run_command(const AST::Command& command)
     StringBuilder cmd;
     cmd.join(" ", command.argv);
 
-    auto job = Job::create(child, pgid, cmd.build(), find_last_job_id() + 1, AST::Command(command));
+    auto command_copy = AST::Command(command);
+    // Clear the next chain if it's to be immediately executed
+    // as the child will run this chain.
+    if (command.should_immediately_execute_next)
+        command_copy.next_chain.clear();
+    auto job = Job::create(child, pgid, cmd.build(), find_last_job_id() + 1, move(command_copy));
     jobs.set((u64)child, job);
 
     job->on_exit = [this](auto job) {
@@ -656,22 +667,25 @@ RefPtr<Job> Shell::run_command(const AST::Command& command)
 
 void Shell::run_tail(const AST::NodeWithAction& next_in_chain, int head_exit_code)
 {
+    auto evaluate = [&] {
+        if (next_in_chain.node->would_execute()) {
+            next_in_chain.node->run(*this);
+            return;
+        }
+        auto commands = next_in_chain.node->to_lazy_evaluated_commands(*this);
+        run_commands(commands);
+    };
     switch (next_in_chain.action) {
     case AST::NodeWithAction::And:
-        if (head_exit_code == 0) {
+        if (head_exit_code == 0)
-            auto commands = next_in_chain.node->run(*this)->resolve_as_commands(*this);
+            evaluate();
-            run_commands(commands);
-        }
         break;
     case AST::NodeWithAction::Or:
-        if (head_exit_code != 0) {
+        if (head_exit_code != 0)
-            auto commands = next_in_chain.node->run(*this)->resolve_as_commands(*this);
+            evaluate();
-            run_commands(commands);
-        }
         break;
     case AST::NodeWithAction::Sequence:
-        auto commands = next_in_chain.node->run(*this)->resolve_as_commands(*this);
+        evaluate();
-        run_commands(commands);
         break;
     }
 }
@@ -745,6 +759,8 @@ bool Shell::run_file(const String& filename, bool explicitly_invoked)
 }
 void Shell::restore_ios()
 {
+    if (m_is_subshell)
+        return;
     tcsetattr(0, TCSANOW, &termios);
     tcsetpgrp(STDOUT_FILENO, m_pid);
     tcsetpgrp(STDIN_FILENO, m_pid);

Shell/Shell.h

@@ -73,6 +73,8 @@ public:
     constexpr static auto local_init_file_path = "~/.shellrc";
     constexpr static auto global_init_file_path = "/etc/shellrc";
 
+    void setup_signals();
+
     int run_command(const StringView&);
     bool is_runnable(const StringView&);
     RefPtr<Job> run_command(const AST::Command&);
@@ -224,6 +226,7 @@ private:
 
     HashMap<String, String> m_aliases;
     bool m_is_interactive { true };
+    bool m_is_subshell { false };
 };
 
 static constexpr bool is_word_character(char c)

Shell/main.cpp

@@ -88,28 +88,8 @@ SavedFileDescriptors::~SavedFileDescriptors()
     }
 }
 
-int main(int argc, char** argv)
+void Shell::setup_signals()
 {
-    Core::EventLoop loop;
-
-    Core::EventLoop::register_signal(SIGINT, [](int) {
-        s_shell->kill_job(s_shell->current_job(), SIGINT);
-    });
-
-    Core::EventLoop::register_signal(SIGWINCH, [](int) {
-        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();
-    });
-
     Core::EventLoop::register_signal(SIGCHLD, [](int) {
         auto& jobs = s_shell->jobs;
         Vector<u64> disowned_jobs;
@@ -159,6 +139,31 @@ int main(int argc, char** argv)
             job->unblock();
         }
     });
+}
+
+int main(int argc, char** argv)
+{
+    Core::EventLoop loop;
+
+    Core::EventLoop::register_signal(SIGINT, [](int) {
+        s_shell->kill_job(s_shell->current_job(), SIGINT);
+    });
+
+    Core::EventLoop::register_signal(SIGWINCH, [](int) {
+        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();
+    });
+
+    s_shell->setup_signals();
 
 #ifndef __serenity__
     sigset_t blocked;
@@ -168,7 +173,7 @@ int main(int argc, char** argv)
     pthread_sigmask(SIG_BLOCK, &blocked, NULL);
 #endif
 #ifdef __serenity__
-    if (pledge("stdio rpath wpath cpath proc exec tty accept sigaction", nullptr) < 0) {
+    if (pledge("stdio rpath wpath cpath proc exec tty accept sigaction unix fattr", nullptr) < 0) {
         perror("pledge");
         return 1;
     }