Thread: Return a result from block() indicating why the block terminated

And use this to return EINTR in various places; some of which we were
not handling properly before.

This might expose a few bugs in userspace, but should be more compatible
with other POSIX systems, and is certainly a little cleaner.

Kernel/Devices/SB16.cpp

@@ -139,7 +139,9 @@ void SB16::handle_irq()
 
 void SB16::wait_for_irq()
 {
-    current->block_until("Interrupting", [this] {
+    // Well, we have no way of knowing how much got written. So just hope all of
+    // it did, even if we're interrupted.
+    (void)current->block_until("Interrupting", [this] {
         return m_interrupted;
     });
 }

Kernel/Net/IPv4Socket.cpp

@@ -212,10 +212,13 @@ ssize_t IPv4Socket::recvfrom(FileDescription& description, void* buffer, size_t
         }
 
         load_receive_deadline();
-        current->block<Thread::ReceiveBlocker>(description);
+        auto res = current->block<Thread::ReceiveBlocker>(description);
 
         LOCKER(lock());
         if (!m_can_read) {
+            if (res == Thread::BlockResult::InterruptedBySignal)
+                return -EINTR;
+
             // Unblocked due to timeout.
             return -EAGAIN;
         }

Kernel/Net/NetworkTask.cpp

@@ -59,7 +59,7 @@ void NetworkTask_main()
     for (;;) {
         auto packet = dequeue_packet();
         if (packet.is_null()) {
-            current->block_until("Networking", [] {
+            (void)current->block_until("Networking", [] {
                 if (LoopbackAdapter::the().has_queued_packets())
                     return true;
                 if (auto* e1000 = E1000NetworkAdapter::the()) {

Kernel/Net/TCPSocket.cpp

@@ -162,7 +162,8 @@ KResult TCPSocket::protocol_connect(FileDescription& description, ShouldBlock sh
     m_state = State::Connecting;
 
     if (should_block == ShouldBlock::Yes) {
-        current->block<Thread::ConnectBlocker>(description);
+        if (current->block<Thread::ConnectBlocker>(description) == Thread::BlockResult::InterruptedBySignal)
+            return KResult(-EINTR);
         ASSERT(is_connected());
         return KSuccess;
     }

Kernel/Process.cpp

@@ -878,9 +878,10 @@ ssize_t Process::sys$writev(int fd, const struct iovec* iov, int iov_count)
     }
 
     if (current->has_unmasked_pending_signals()) {
-        current->block<Thread::SemiPermanentBlocker>(Thread::SemiPermanentBlocker::Reason::Signal);
+        if (current->block<Thread::SemiPermanentBlocker>(Thread::SemiPermanentBlocker::Reason::Signal) == Thread::BlockResult::InterruptedBySignal) {
-        if (nwritten == 0)
+            if (nwritten == 0)
-            return -EINTR;
+                return -EINTR;
+        }
     }
 
     return nwritten;
@@ -909,7 +910,10 @@ ssize_t Process::do_write(FileDescription& description, const u8* data, int data
 #ifdef IO_DEBUG
             dbgprintf("block write on %d\n", fd);
 #endif
-            current->block<Thread::WriteBlocker>(description);
+            if (current->block<Thread::WriteBlocker>(description) == Thread::BlockResult::InterruptedBySignal) {
+                if (nwritten == 0)
+                    return -EINTR;
+            }
         }
         ssize_t rc = description.write(data + nwritten, data_size - nwritten);
 #ifdef IO_DEBUG
@@ -923,9 +927,10 @@ ssize_t Process::do_write(FileDescription& description, const u8* data, int data
         if (rc == 0)
             break;
         if (current->has_unmasked_pending_signals()) {
-            current->block<Thread::SemiPermanentBlocker>(Thread::SemiPermanentBlocker::Reason::Signal);
+            if (current->block<Thread::SemiPermanentBlocker>(Thread::SemiPermanentBlocker::Reason::Signal) == Thread::BlockResult::InterruptedBySignal) {
-            if (nwritten == 0)
+                if (nwritten == 0)
-                return -EINTR;
+                    return -EINTR;
+            }
         }
         nwritten += rc;
     }
@@ -948,9 +953,10 @@ ssize_t Process::sys$write(int fd, const u8* data, ssize_t size)
         return -EBADF;
     auto nwritten = do_write(*description, data, size);
     if (current->has_unmasked_pending_signals()) {
-        current->block<Thread::SemiPermanentBlocker>(Thread::SemiPermanentBlocker::Reason::Signal);
+        if (current->block<Thread::SemiPermanentBlocker>(Thread::SemiPermanentBlocker::Reason::Signal) == Thread::BlockResult::InterruptedBySignal) {
-        if (nwritten == 0)
+            if (nwritten == 0)
-            return -EINTR;
+                return -EINTR;
+        }
     }
     return nwritten;
 }
@@ -971,8 +977,7 @@ ssize_t Process::sys$read(int fd, u8* buffer, ssize_t size)
         return -EBADF;
     if (description->is_blocking()) {
         if (!description->can_read()) {
-            current->block<Thread::ReadBlocker>(*description);
+            if (current->block<Thread::ReadBlocker>(*description) == Thread::BlockResult::InterruptedBySignal)
-            if (current->m_was_interrupted_while_blocked)
                 return -EINTR;
         }
     }
@@ -1274,7 +1279,7 @@ int Process::sys$kill(pid_t pid, int signal)
     }
     if (pid == m_pid) {
         current->send_signal(signal, this);
-        current->block<Thread::SemiPermanentBlocker>(Thread::SemiPermanentBlocker::Reason::Signal);
+        (void)current->block<Thread::SemiPermanentBlocker>(Thread::SemiPermanentBlocker::Reason::Signal);
         return 0;
     }
     InterruptDisabler disabler;
@@ -1300,7 +1305,6 @@ int Process::sys$usleep(useconds_t usec)
 
     u64 wakeup_time = current->sleep(usec / 1000);
     if (wakeup_time > g_uptime) {
-        ASSERT(current->m_was_interrupted_while_blocked);
         u32 ticks_left_until_original_wakeup_time = wakeup_time - g_uptime;
         return ticks_left_until_original_wakeup_time / TICKS_PER_SECOND;
     }
@@ -1313,7 +1317,6 @@ int Process::sys$sleep(unsigned seconds)
         return 0;
     u64 wakeup_time = current->sleep(seconds * TICKS_PER_SECOND);
     if (wakeup_time > g_uptime) {
-        ASSERT(current->m_was_interrupted_while_blocked);
         u32 ticks_left_until_original_wakeup_time = wakeup_time - g_uptime;
         return ticks_left_until_original_wakeup_time / TICKS_PER_SECOND;
     }
@@ -1451,8 +1454,7 @@ pid_t Process::sys$waitpid(pid_t waitee, int* wstatus, int options)
     }
 
     pid_t waitee_pid = waitee;
-    current->block<Thread::WaitBlocker>(options, waitee_pid);
+    if (current->block<Thread::WaitBlocker>(options, waitee_pid) == Thread::BlockResult::InterruptedBySignal)
-    if (current->m_was_interrupted_while_blocked)
         return -EINTR;
 
     InterruptDisabler disabler;
@@ -1835,8 +1837,9 @@ int Process::sys$select(const Syscall::SC_select_params* params)
     dbgprintf("%s<%u> selecting on (read:%u, write:%u), timeout=%p\n", name().characters(), pid(), rfds.size(), wfds.size(), params->timeout);
 #endif
 
+    Thread::BlockResult block_res = Thread::BlockResult::WokeNormally;
     if (!params->timeout || select_has_timeout)
-        current->block<Thread::SelectBlocker>(timeout, select_has_timeout, rfds, wfds, efds);
+        block_res = current->block<Thread::SelectBlocker>(timeout, select_has_timeout, rfds, wfds, efds);
 
     int marked_fd_count = 0;
     auto mark_fds = [&](auto* fds, auto& vector, auto should_mark) {
@@ -1853,6 +1856,12 @@ int Process::sys$select(const Syscall::SC_select_params* params)
     mark_fds(params->readfds, rfds, [](auto& description) { return description.can_read(); });
     mark_fds(params->writefds, wfds, [](auto& description) { return description.can_write(); });
     // FIXME: We should also mark params->exceptfds as appropriate.
+
+    if (marked_fd_count == 0) {
+        if (block_res == Thread::BlockResult::InterruptedBySignal)
+            return -EINTR;
+    }
+
     return marked_fd_count;
 }
 
@@ -1890,8 +1899,9 @@ int Process::sys$poll(pollfd* fds, int nfds, int timeout)
     dbgprintf("%s<%u> polling on (read:%u, write:%u), timeout=%d\n", name().characters(), pid(), rfds.size(), wfds.size(), timeout);
 #endif
 
+    Thread::BlockResult block_res = Thread::BlockResult::WokeNormally;
     if (has_timeout|| timeout < 0)
-        current->block<Thread::SelectBlocker>(actual_timeout, has_timeout, rfds, wfds, Thread::SelectBlocker::FDVector());
+        block_res = current->block<Thread::SelectBlocker>(actual_timeout, has_timeout, rfds, wfds, Thread::SelectBlocker::FDVector());
 
     int fds_with_revents = 0;
 
@@ -1911,6 +1921,11 @@ int Process::sys$poll(pollfd* fds, int nfds, int timeout)
             ++fds_with_revents;
     }
 
+    if (fds_with_revents == 0) {
+        if (block_res != Thread::BlockResult::InterruptedBySignal)
+            return -EINTR;
+    }
+
     return fds_with_revents;
 }
 
@@ -2134,8 +2149,7 @@ int Process::sys$accept(int accepting_socket_fd, sockaddr* address, socklen_t* a
     auto& socket = *accepting_socket_description->socket();
     if (!socket.can_accept()) {
         if (accepting_socket_description->is_blocking()) {
-            current->block<Thread::AcceptBlocker>(*accepting_socket_description);
+            if (current->block<Thread::AcceptBlocker>(*accepting_socket_description) == Thread::BlockResult::InterruptedBySignal)
-            if (current->m_was_interrupted_while_blocked)
                 return -EINTR;
         } else {
             return -EAGAIN;

Kernel/Scheduler.cpp

@@ -309,7 +309,8 @@ bool Scheduler::pick_next()
             return IterationDecision::Continue;
         if (was_blocked) {
             dbgprintf("Unblock %s(%u) due to signal\n", thread.process().name().characters(), thread.pid());
-            thread.m_was_interrupted_while_blocked = true;
+            ASSERT(thread.m_blocker);
+            thread.m_blocker->set_interrupted_by_signal();
             thread.unblock();
         }
         return IterationDecision::Continue;

Kernel/Thread.cpp

@@ -106,16 +106,10 @@ void Thread::unblock()
     set_state(Thread::Runnable);
 }
 
-void Thread::block_until(const char* state_string, Function<bool()>&& condition)
-{
-    block<ConditionBlocker>(state_string, move(condition));
-}
-
 void Thread::block_helper()
 {
     bool did_unlock = process().big_lock().unlock_if_locked();
     ASSERT(state() == Thread::Running);
-    m_was_interrupted_while_blocked = false;
     set_state(Thread::Blocked);
     Scheduler::yield();
     if (did_unlock)
@@ -126,7 +120,10 @@ u64 Thread::sleep(u32 ticks)
 {
     ASSERT(state() == Thread::Running);
     u64 wakeup_time = g_uptime + ticks;
-    current->block<Thread::SleepBlocker>(wakeup_time);
+    auto ret = current->block<Thread::SleepBlocker>(wakeup_time);
+    if (wakeup_time > g_uptime) {
+        ASSERT(ret == Thread::BlockResult::InterruptedBySignal);
+    }
     return wakeup_time;
 }
 
@@ -522,7 +519,8 @@ KResult Thread::wait_for_connect(FileDescription& description)
     auto& socket = *description.socket();
     if (socket.is_connected())
         return KSuccess;
-    block<Thread::ConnectBlocker>(description);
+    if (block<Thread::ConnectBlocker>(description) == Thread::BlockResult::InterruptedBySignal)
+        return KResult(-EINTR);
     Scheduler::yield();
     if (!socket.is_connected())
         return KResult(-ECONNREFUSED);

Kernel/Thread.h

@@ -68,6 +68,10 @@ public:
         virtual ~Blocker() {}
         virtual bool should_unblock(Thread&, time_t now_s, long us) = 0;
         virtual const char* state_string() const = 0;
+        void set_interrupted_by_signal() { m_was_interrupted_while_blocked = true; }
+        bool was_interrupted_by_signal() const { return m_was_interrupted_while_blocked; }
+    private:
+        bool m_was_interrupted_while_blocked { false };
     };
 
     class FileDescriptionBlocker : public Blocker {
@@ -205,17 +209,30 @@ public:
 
     u64 sleep(u32 ticks);
 
+    enum class BlockResult {
+        WokeNormally,
+        InterruptedBySignal,
+    };
+
     template <typename T, class... Args>
-    void block(Args&& ... args)
+    [[nodiscard]] BlockResult block(Args&& ... args)
     {
         ASSERT(!m_blocker);
         T t(AK::forward<Args>(args)...);
         m_blocker = &t;
         block_helper();
+        if (t.was_interrupted_by_signal())
+            return BlockResult::InterruptedBySignal;
+        return BlockResult::WokeNormally;
     };
 
+    [[nodiscard]] BlockResult block_until(const char* state_string, Function<bool()>&& condition)
+    {
+        return block<ConditionBlocker>(state_string, move(condition));
+    }
+
     void unblock();
-    void block_until(const char* state_string, Function<bool()>&&);
+
     KResult wait_for_connect(FileDescription&);
 
     const FarPtr& far_ptr() const { return m_far_ptr; }
@@ -301,7 +318,6 @@ private:
     FPUState* m_fpu_state { nullptr };
     State m_state { Invalid };
     bool m_has_used_fpu { false };
-    bool m_was_interrupted_while_blocked { false };
 
     void block_helper();
 };

Kernel/init.cpp

@@ -242,7 +242,7 @@ extern "C" [[noreturn]] void init()
         current->process().set_priority(Process::LowPriority);
         for (;;) {
             Thread::finalize_dying_threads();
-            current->block<Thread::SemiPermanentBlocker>(Thread::SemiPermanentBlocker::Reason::Lurking);
+            (void)current->block<Thread::SemiPermanentBlocker>(Thread::SemiPermanentBlocker::Reason::Lurking);
             Scheduler::yield();
         }
     });