Kernel: Make waitpid() block by default unless WNOHANG is specified

apps/src/init.c

@@ -60,10 +60,7 @@ int main()
 
     for (;;)
     {
-        while ((result = wait(NULL)) == 0) // No child has exited yet
+        result = wait(NULL);
-        {
-            msleep(100);
-        }
         if (result == child) return 0;
     }
 }

apps/src/sh.c

@@ -169,8 +169,7 @@ void command_execute(command* cmd)
         perror(argv[0]);
         exit(127);
     }
-    pid_t result;
+    pid_t result = waitpid(child, &status, 0);
-    while ((result = waitpid(child, &status, 0)) == 0) { msleep(20); }
     if (result < 0)
     {
         perror("waitpid");

kernel/include/thread/Task.h

@@ -7,6 +7,13 @@
 
 #define TASK_MAX_FDS 32
 
+enum class BlockReason
+{
+    None,
+    Reading,
+    Waiting,
+};
+
 struct Task
 {
     enum TaskState
@@ -69,16 +76,32 @@ struct Task
 
     char name[128];
 
-    struct
+    BlockReason block_reason;
-    {
-        size_t size;
-        int fd;
-        char* buf;
-    } blocking_read_info;
 
-    void resume_read();
+    union {
+        struct
+        {
+            size_t size;
+            int fd;
+            char* buf;
+        } blocking_read_info;
+        struct
+        {
+            int64_t wait_pid;
+            int* wstatus;
+        } blocking_wait_info;
+    };
+
+    void resume();
 
     bool is_still_blocking();
 
     Descriptor* descriptor_from_fd(int fd, int& error);
+
+  private:
+    void resume_read();
+    void resume_wait();
+
+    bool is_read_still_blocking();
+    bool is_wait_still_blocking();
 };

kernel/src/sys/stdio.cpp

@@ -255,6 +255,7 @@ void sys_read(Context* context, int fd, size_t size, char* buffer)
         }
         Task* current_task = Scheduler::current_task();
         current_task->state = current_task->Blocking;
+        current_task->block_reason = BlockReason::Reading;
         current_task->blocking_read_info.fd = fd;
         current_task->blocking_read_info.buf = (char*)VMM::get_physical((uint64_t)buffer); // FIXME: Handle errors.
         current_task->blocking_read_info.size = size;

kernel/src/thread/Scheduler.cpp

@@ -404,11 +404,7 @@ void Scheduler::task_yield(Context* context)
         sched_current_task = sched_current_task->next_task;
         if (sched_current_task->state == sched_current_task->Blocking)
         {
-            if (!sched_current_task->is_still_blocking())
+            if (!sched_current_task->is_still_blocking()) sched_current_task->resume();
-            {
-                sched_current_task->resume_read();
-                sched_current_task->state = sched_current_task->Running;
-            }
         }
         if (sched_current_task->state == sched_current_task->Running)
         {
@@ -464,8 +460,10 @@ Task* Scheduler::current_task()
     return sched_current_task;
 }
 
+#define WNOHANG 1
+
 void sys_waitpid(Context* context, long pid, int* wstatus,
-                 int) // FIXME: Use the value in options and block if WNOHANG has not been specified.
+                 int options) // FIXME: only allow waiting for child processes when specifying a PID.
 {
     Task* child = nullptr;
     if (pid == -1)
@@ -480,8 +478,29 @@ void sys_waitpid(Context* context, long pid, int* wstatus,
         });
         if (!child)
         {
-            context->rax = 0; // No child has exited, let's return 0.
+            if (options & WNOHANG)
-            return;
+            {
+                context->rax = 0; // No child has exited, let's return 0.
+                return;
+            }
+            int* kwstatus;
+            if (wstatus)
+            {
+                kwstatus = obtain_user_ref(wstatus);
+                if (!kwstatus)
+                {
+                    context->rax = -EFAULT;
+                    return;
+                }
+            }
+            kdbgln("blocking wait on any child");
+            sched_current_task->state = sched_current_task->Blocking;
+            sched_current_task->block_reason = BlockReason::Waiting;
+            sched_current_task->blocking_wait_info.wait_pid = -1;
+            if (wstatus) sched_current_task->blocking_wait_info.wstatus = kwstatus;
+            else
+                sched_current_task->blocking_wait_info.wstatus = nullptr;
+            return Scheduler::task_yield(context);
         }
     }
     else
@@ -493,10 +512,31 @@ void sys_waitpid(Context* context, long pid, int* wstatus,
             return;
         }
     }
-    if (child->state != child->Dying) // FIXME: This should block if WNOHANG has not been specified.
+    if (child->state != child->Dying)
     {
-        context->rax = 0;
+        if (options & WNOHANG)
-        return;
+        {
+            context->rax = 0; // No child has exited, let's return 0.
+            return;
+        }
+        int* kwstatus;
+        if (wstatus)
+        {
+            kwstatus = obtain_user_ref(wstatus);
+            if (!kwstatus)
+            {
+                context->rax = -EFAULT;
+                return;
+            }
+        }
+        kdbgln("blocking wait on pid %ld", pid);
+        sched_current_task->state = sched_current_task->Blocking;
+        sched_current_task->block_reason = BlockReason::Waiting;
+        sched_current_task->blocking_wait_info.wait_pid = pid;
+        if (wstatus) sched_current_task->blocking_wait_info.wstatus = kwstatus;
+        else
+            sched_current_task->blocking_wait_info.wstatus = nullptr;
+        return Scheduler::task_yield(context);
     }
     if (wstatus)
     {
@@ -518,6 +558,54 @@ void sys_waitpid(Context* context, long pid, int* wstatus,
     context->rax = (long)child->id;
 }
 
+bool Task::is_wait_still_blocking()
+{
+    Task* child = nullptr;
+    if (blocking_wait_info.wait_pid == -1)
+    {
+        sched_for_each_child(sched_current_task, [&](Task* task) {
+            if (task->state == task->Dying)
+            {
+                child = task;
+                return false;
+            }
+            return true;
+        });
+        if (!child) return true;
+        else
+        {
+            blocking_wait_info.wait_pid = child->id; // We're committed to this child now.
+            return false;
+        }
+    }
+    else
+    {
+        child = Scheduler::find_by_pid(blocking_wait_info.wait_pid);
+        if (child->state != child->Dying) return true;
+        else
+            return false;
+    }
+}
+
+void Task::resume_wait()
+{
+    ASSERT(blocking_wait_info.wait_pid != -1); // is_wait_still_blocking should have chosen a child for us if the user
+                                               // process told us to wait for any child.
+    Task* child = Scheduler::find_by_pid(blocking_wait_info.wait_pid);
+    ASSERT(child); // This should also already have been validated.
+
+    kdbgln("resuming wait on child %ld", child->id);
+
+    if (blocking_wait_info.wstatus)
+    {
+        *blocking_wait_info.wstatus = (int)(child->exit_status & 0xff);
+        release_user_ref(blocking_wait_info.wstatus);
+    }
+
+    child->state = child->Exited;
+    regs.rax = (long)child->id;
+}
+
 struct pstat
 {
     long pt_pid;

kernel/src/thread/Task.cpp

@@ -3,6 +3,7 @@
 #include "thread/Task.h"
 #include "log/Log.h"
 #include "memory/VMM.h"
+#include "std/assert.h"
 #include "std/errno.h"
 #include "std/string.h"
 
@@ -80,11 +81,37 @@ void Task::resume_read()
     VMM::apply_address_space();
 }
 
-bool Task::is_still_blocking()
+bool Task::is_read_still_blocking()
 {
     return VFS::would_block(files[blocking_read_info.fd].node());
 }
 
+void Task::resume()
+{
+    switch (block_reason)
+    {
+    case BlockReason::None: ASSERT(false);
+    case BlockReason::Reading: resume_read(); break;
+    case BlockReason::Waiting: resume_wait(); break;
+
+    default: ASSERT(false);
+    }
+    block_reason = BlockReason::None;
+    state = Running;
+}
+
+bool Task::is_still_blocking()
+{
+    switch (block_reason)
+    {
+    case BlockReason::None: ASSERT(false);
+    case BlockReason::Reading: return is_read_still_blocking();
+    case BlockReason::Waiting: return is_wait_still_blocking();
+
+    default: ASSERT(false);
+    }
+}
+
 Descriptor* Task::descriptor_from_fd(int fd, int& error)
 {
     if (fd < 0 || fd >= TASK_MAX_FDS)

libs/libc/include/sys/wait.h

@@ -9,6 +9,9 @@
 /* What was the child's exit status? */
 #define WEXITSTATUS(status) (char)((status)&0xff)
 
+/* Do not block the current process if no child has exited. */
+#define WNOHANG 1
+
 #ifdef __cplusplus
 extern "C"
 {