diff --git a/kernel/include/kernel/Process.h b/kernel/include/kernel/Process.h index 54a2d4ab..f65ba0c1 100644 --- a/kernel/include/kernel/Process.h +++ b/kernel/include/kernel/Process.h @@ -219,8 +219,6 @@ namespace Kernel // Load elf from a file static BAN::ErrorOr> load_elf_for_exec(const Credentials&, BAN::StringView file_path, const BAN::String& cwd, Kernel::PageTable&); - BAN::ErrorOr block_until_exit(pid_t pid); - BAN::ErrorOr validate_string_access(const char*); BAN::ErrorOr validate_pointer_access_check(const void*, size_t); BAN::ErrorOr validate_pointer_access(const void*, size_t); @@ -255,12 +253,12 @@ namespace Kernel } private: - struct ExitStatus + struct ChildExitStatus { - ThreadBlocker thread_blocker; - int exit_code { 0 }; - BAN::Atomic exited { false }; - BAN::Atomic waiting { 0 }; + pid_t pid { 0 }; + pid_t pgrp { 0 }; + int exit_code { 0 }; + bool exited { false }; }; Credentials m_credentials; @@ -292,7 +290,10 @@ namespace Kernel bool m_is_userspace { false }; userspace_info_t m_userspace_info; - ExitStatus m_exit_status; + + SpinLock m_child_exit_lock; + BAN::Vector m_child_exit_statuses; + ThreadBlocker m_child_exit_blocker; bool m_has_called_exec { false }; diff --git a/kernel/include/kernel/Thread.h b/kernel/include/kernel/Thread.h index c8520686..5813ae91 100644 --- a/kernel/include/kernel/Thread.h +++ b/kernel/include/kernel/Thread.h @@ -49,6 +49,8 @@ namespace Kernel bool add_signal(int signal); // blocks current thread and returns either on unblock, eintr, spuriously or after timeout + BAN::ErrorOr sleep_or_eintr_ms(uint64_t ms) { return sleep_or_eintr_ns(ms * 1'000'000); } + BAN::ErrorOr sleep_or_eintr_ns(uint64_t ns); BAN::ErrorOr block_or_eintr_indefinite(ThreadBlocker& thread_blocker); BAN::ErrorOr block_or_eintr_or_timeout_ms(ThreadBlocker& thread_blocker, uint64_t timeout_ms, bool etimedout) { return block_or_eintr_or_timeout_ns(thread_blocker, timeout_ms * 1'000'000, etimedout); } BAN::ErrorOr block_or_eintr_or_waketime_ms(ThreadBlocker& thread_blocker, uint64_t wake_time_ms, bool etimedout) { return block_or_eintr_or_waketime_ns(thread_blocker, wake_time_ms * 1'000'000, etimedout); } diff --git a/kernel/kernel/Process.cpp b/kernel/kernel/Process.cpp index 35c535e4..78120e6b 100644 --- a/kernel/kernel/Process.cpp +++ b/kernel/kernel/Process.cpp @@ -185,7 +185,6 @@ namespace Kernel ASSERT(m_threads.empty()); ASSERT(m_mapped_regions.empty()); ASSERT(!m_loadable_elf); - ASSERT(m_exit_status.waiting == 0); ASSERT(&PageTable::current() != m_page_table.ptr()); } @@ -201,21 +200,15 @@ namespace Kernel SpinLockGuard _(s_process_lock); for (size_t i = 0; i < s_processes.size(); i++) { - if (m_parent && s_processes[i]->pid() == m_parent) - s_processes[i]->add_pending_signal(SIGCHLD); - if (s_processes[i] == this) - s_processes.remove(i); + if (s_processes[i] != this) + continue; + s_processes.remove(i); + break; } } ProcFileSystem::get().on_process_delete(*this); - m_exit_status.exited = true; - m_exit_status.thread_blocker.unblock(); - - while (m_exit_status.waiting > 0) - Processor::yield(); - m_process_lock.lock(); m_open_file_descriptors.close_all(); @@ -252,7 +245,35 @@ namespace Kernel void Process::exit(int status, int signal) { - m_exit_status.exit_code = __WGENEXITCODE(status, signal); + if (m_parent) + { + for_each_process( + [&](Process& parent) -> BAN::Iteration + { + if (parent.pid() != m_parent) + return BAN::Iteration::Continue; + + for (auto& child : parent.m_child_exit_statuses) + { + if (child.pid != pid()) + continue; + + child.exit_code = __WGENEXITCODE(status, signal); + child.exited = true; + + parent.add_pending_signal(SIGCHLD); + Processor::scheduler().unblock_thread(parent.m_threads.front()); + + parent.m_child_exit_blocker.unblock(); + + break; + } + + return BAN::Iteration::Break; + } + ); + } + while (!m_threads.empty()) m_threads.front()->on_exit(); } @@ -397,6 +418,20 @@ namespace Kernel LockGuard _(m_process_lock); + ChildExitStatus* child_exit_status = nullptr; + for (auto& child : m_child_exit_statuses) + { + if (child.pid != 0) + continue; + child_exit_status = &child; + break; + } + if (child_exit_status == nullptr) + { + TRY(m_child_exit_statuses.emplace_back()); + child_exit_status = &m_child_exit_statuses.back(); + } + BAN::String working_directory; TRY(working_directory.append(m_working_directory)); @@ -422,6 +457,10 @@ namespace Kernel forked->m_has_called_exec = false; memcpy(forked->m_signal_handlers, m_signal_handlers, sizeof(m_signal_handlers)); + *child_exit_status = {}; + child_exit_status->pid = forked->pid(); + child_exit_status->pgrp = forked->pgrp(); + ASSERT(this == &Process::current()); // FIXME: this should be able to fail Thread* thread = MUST(Thread::current().clone(forked, sp, ip)); @@ -541,58 +580,72 @@ namespace Kernel ASSERT_NOT_REACHED(); } - BAN::ErrorOr Process::block_until_exit(pid_t pid) - { - ASSERT(this->pid() != pid); - - Process* target = nullptr; - for_each_process( - [pid, &target](Process& process) - { - if (process.pid() == pid) - { - process.m_exit_status.waiting++; - target = &process; - return BAN::Iteration::Break; - } - return BAN::Iteration::Continue; - } - ); - - if (target == nullptr) - return BAN::Error::from_errno(ECHILD); - - while (!target->m_exit_status.exited) - { - if (auto ret = Thread::current().block_or_eintr_indefinite(target->m_exit_status.thread_blocker); ret.is_error()) - { - target->m_exit_status.waiting--; - return ret.release_error(); - } - } - - int exit_status = target->m_exit_status.exit_code; - target->m_exit_status.waiting--; - - return exit_status; - } - BAN::ErrorOr Process::sys_wait(pid_t pid, int* stat_loc, int options) { - { - LockGuard _(m_process_lock); - TRY(validate_pointer_access(stat_loc, sizeof(int))); - } - - // FIXME: support options - if (options) + if (options & ~(WCONTINUED | WNOHANG | WUNTRACED)) return BAN::Error::from_errno(EINVAL); - int stat = TRY(block_until_exit(pid)); - if (stat_loc) - *stat_loc = stat; + // FIXME: support options stopped processes + if (options & ~(WCONTINUED | WUNTRACED)) + return BAN::Error::from_errno(ENOTSUP); - return pid; + const auto pid_matches = + [&](const ChildExitStatus& child) + { + if (pid == -1) + return true; + if (pid == 0) + return child.pgrp == pgrp(); + if (pid < 0) + return child.pgrp == -pid; + return child.pid == pid; + }; + + for (;;) + { + pid_t exited_pid = 0; + int exit_code = 0; + { + SpinLockGuard _(m_child_exit_lock); + + bool found = false; + for (auto& child : m_child_exit_statuses) + { + if (!pid_matches(child)) + continue; + found = true; + if (!child.exited) + continue; + exited_pid = child.pid; + exit_code = child.exit_code; + child = {}; + break; + } + + if (!found) + return BAN::Error::from_errno(ECHILD); + } + + if (exited_pid != 0) + { + if (stat_loc) + { + LockGuard _(m_process_lock); + TRY(validate_pointer_access(stat_loc, sizeof(stat_loc))); + *stat_loc = exit_code; + } + remove_pending_signal(SIGCHLD); + return exited_pid; + } + + if (Thread::current().is_interrupted_by_signal()) + return BAN::Error::from_errno(EINTR); + + if (options & WNOHANG) + return 0; + + m_child_exit_blocker.block_indefinite(); + } } BAN::ErrorOr Process::sys_sleep(int seconds) @@ -1289,7 +1342,7 @@ namespace Kernel break; LockFreeGuard free(m_process_lock); - SystemTimer::get().sleep_ms(1); + TRY(Thread::current().sleep_or_eintr_ms(1)); } if (arguments->readfds) diff --git a/kernel/kernel/Thread.cpp b/kernel/kernel/Thread.cpp index a9492c6d..fd29666f 100644 --- a/kernel/kernel/Thread.cpp +++ b/kernel/kernel/Thread.cpp @@ -423,6 +423,16 @@ namespace Kernel return false; } + BAN::ErrorOr Thread::sleep_or_eintr_ns(uint64_t ns) + { + if (is_interrupted_by_signal()) + return BAN::Error::from_errno(EINTR); + SystemTimer::get().sleep_ns(ns); + if (is_interrupted_by_signal()) + return BAN::Error::from_errno(EINTR); + return {}; + } + BAN::ErrorOr Thread::block_or_eintr_indefinite(ThreadBlocker& thread_blocker) { if (is_interrupted_by_signal()) diff --git a/userspace/programs/Shell/main.cpp b/userspace/programs/Shell/main.cpp index 1067242d..a61a673e 100644 --- a/userspace/programs/Shell/main.cpp +++ b/userspace/programs/Shell/main.cpp @@ -147,7 +147,7 @@ BAN::Optional parse_dollar(BAN::StringView command, size_t& i) close(fds[0]); int status; - if (waitpid(pid, &status, 0) == -1 && errno != ECHILD) + if (waitpid(pid, &status, 0) == -1) ERROR_RETURN("waitpid", {}); while (!output.empty() && output.back() == '\n')