Kernel: Move Blocker setup out from constructors into setup_blocker()

Instead of registering with blocker sets and whatnot in the various
Blocker subclass constructors, this patch moves such initialization
to a separate setup_blocker() virtual.

setup_blocker() returns false if there's no need to actually block
the thread. This allows us to bail earlier in Thread::block().

Kernel/FileSystem/Plan9FileSystem.cpp

@@ -425,6 +425,11 @@ bool Plan9FS::Blocker::unblock(u16 tag)
     return unblock();
 }
 
+bool Plan9FS::Blocker::setup_blocker()
+{
+    return add_to_blocker_set(m_fs.m_completion_blocker);
+}
+
 void Plan9FS::Blocker::will_unblock_immediately_without_blocking(UnblockImmediatelyReason)
 {
     {

Kernel/FileSystem/Plan9FileSystem.h

@@ -87,8 +87,8 @@ private:
             , m_message(message)
             , m_completion(move(completion))
         {
-            add_to_blocker_set(fs.m_completion_blocker);
         }
+        virtual bool setup_blocker() override;
         virtual StringView state_string() const override { return "Waiting"sv; }
         virtual Type blocker_type() const override { return Type::Plan9FS; }
         virtual void will_unblock_immediately_without_blocking(UnblockImmediatelyReason) override;

Kernel/Net/Routing.cpp

@@ -25,6 +25,7 @@ public:
     virtual StringView state_string() const override { return "Routing (ARP)"sv; }
     virtual Type blocker_type() const override { return Type::Routing; }
     virtual bool should_block() override { return m_should_block; }
+    virtual bool setup_blocker() override;
 
     virtual void will_unblock_immediately_without_blocking(UnblockImmediatelyReason) override;
 
@@ -85,9 +86,14 @@ static Singleton<ARPTableBlockerSet> s_arp_table_blocker_set;
 ARPTableBlocker::ARPTableBlocker(IPv4Address ip_addr, Optional<MACAddress>& addr)
     : m_ip_addr(ip_addr)
     , m_addr(addr)
+{
+}
+
+bool ARPTableBlocker::setup_blocker()
 {
     if (!add_to_blocker_set(*s_arp_table_blocker_set))
         m_should_block = false;
+    return m_should_block;
 }
 
 void ARPTableBlocker::will_unblock_immediately_without_blocking(UnblockImmediatelyReason reason)

Kernel/Thread.h

@@ -300,6 +300,7 @@ public:
         virtual Type blocker_type() const = 0;
         virtual const BlockTimeout& override_timeout(const BlockTimeout& timeout) { return timeout; }
         virtual bool can_be_interrupted() const { return true; }
+        virtual bool setup_blocker();
 
         Thread& thread() { return m_thread; }
 
@@ -523,11 +524,14 @@ public:
         virtual bool should_block() override { return !m_join_error && m_should_block; }
         virtual void will_unblock_immediately_without_blocking(UnblockImmediatelyReason) override;
 
+        virtual bool setup_blocker() override;
+
         bool unblock(void*, bool);
 
     private:
         NonnullRefPtr<Thread> m_joinee;
         void*& m_joinee_exit_value;
+        KResult& m_try_join_result;
         bool m_join_error { false };
         bool m_did_unblock { false };
         bool m_should_block { true };
@@ -541,15 +545,13 @@ public:
         virtual Type blocker_type() const override { return Type::Queue; }
         virtual StringView state_string() const override { return m_block_reason.is_null() ? m_block_reason : "Queue"sv; }
         virtual void will_unblock_immediately_without_blocking(UnblockImmediatelyReason) override { }
-
+        virtual bool should_block() override { return m_should_block; }
-        virtual bool should_block() override
+        virtual bool setup_blocker() override;
-        {
-            return m_should_block;
-        }
 
         bool unblock();
 
     protected:
+        WaitQueue& m_wait_queue;
         StringView m_block_reason;
         bool m_should_block { true };
         bool m_did_unblock { false };
@@ -563,11 +565,8 @@ public:
         virtual Type blocker_type() const override { return Type::Futex; }
         virtual StringView state_string() const override { return "Futex"sv; }
         virtual void will_unblock_immediately_without_blocking(UnblockImmediatelyReason) override { }
-
+        virtual bool should_block() override { return m_should_block; }
-        virtual bool should_block() override
+        virtual bool setup_blocker() override;
-        {
-            return m_should_block;
-        }
 
         u32 bitset() const { return m_bitset; }
 
@@ -582,7 +581,8 @@ public:
         bool unblock(bool force = false);
 
     protected:
-        u32 m_bitset;
+        FutexQueue& m_futex_queue;
+        u32 m_bitset { 0 };
         u32 m_relock_flags { 0 };
         bool m_should_block { true };
         bool m_did_unblock { false };
@@ -627,6 +627,7 @@ public:
 
         virtual bool unblock(bool, void*) override;
         virtual void will_unblock_immediately_without_blocking(UnblockImmediatelyReason) override;
+        virtual bool setup_blocker() override;
 
     protected:
         explicit FileDescriptionBlocker(FileDescription&, BlockFlags, BlockFlags&);
@@ -696,13 +697,14 @@ public:
         };
 
         typedef Vector<FDInfo, FD_SETSIZE> FDVector;
-        explicit SelectBlocker(FDVector& fds);
+        explicit SelectBlocker(FDVector&);
         virtual ~SelectBlocker();
 
         virtual bool unblock(bool, void*) override;
         virtual void will_unblock_immediately_without_blocking(UnblockImmediatelyReason) override;
         virtual void was_unblocked(bool) override;
         virtual StringView state_string() const override { return "Selecting"sv; }
+        virtual bool setup_blocker() override;
 
     private:
         size_t collect_unblocked_flags();
@@ -726,6 +728,7 @@ public:
         virtual bool should_block() override { return m_should_block; }
         virtual void will_unblock_immediately_without_blocking(UnblockImmediatelyReason) override;
         virtual void was_unblocked(bool) override;
+        virtual bool setup_blocker() override;
 
         bool unblock(Process& process, UnblockFlags flags, u8 signal, bool from_add_blocker);
         bool is_wait() const { return !(m_wait_options & WNOWAIT); }
@@ -867,6 +870,12 @@ public:
         m_in_block = true;
         BlockerType blocker(forward<Args>(args)...);
 
+        if (!blocker.setup_blocker()) {
+            blocker.will_unblock_immediately_without_blocking(Blocker::UnblockImmediatelyReason::UnblockConditionAlreadyMet);
+            m_in_block = false;
+            return BlockResult::NotBlocked;
+        }
+
         SpinlockLocker scheduler_lock(g_scheduler_lock);
         // Relaxed semantics are fine for timeout_unblocked because we
         // synchronize on the spin locks already.
@@ -885,13 +894,6 @@ public:
             }
 
             m_blocker = &blocker;
-            if (!blocker.should_block()) {
-                // Don't block if the wake condition is already met
-                blocker.will_unblock_immediately_without_blocking(Blocker::UnblockImmediatelyReason::UnblockConditionAlreadyMet);
-                m_blocker = nullptr;
-                m_in_block = false;
-                return BlockResult::NotBlocked;
-            }
 
             auto& block_timeout = blocker.override_timeout(timeout);
             if (!block_timeout.is_infinite()) {

Kernel/ThreadBlockers.cpp

@@ -45,6 +45,11 @@ Thread::Blocker::~Blocker()
         m_blocker_set->remove_blocker(*this);
 }
 
+bool Thread::Blocker::setup_blocker()
+{
+    return true;
+}
+
 void Thread::Blocker::begin_blocking(Badge<Thread>)
 {
     SpinlockLocker lock(m_lock);
@@ -68,19 +73,23 @@ auto Thread::Blocker::end_blocking(Badge<Thread>, bool did_timeout) -> BlockResu
 Thread::JoinBlocker::JoinBlocker(Thread& joinee, KResult& try_join_result, void*& joinee_exit_value)
     : m_joinee(joinee)
     , m_joinee_exit_value(joinee_exit_value)
+    , m_try_join_result(try_join_result)
 {
-    {
+}
-        // We need to hold our lock to avoid a race where try_join succeeds
+
-        // but the joinee is joining immediately
+bool Thread::JoinBlocker::setup_blocker()
-        SpinlockLocker lock(m_lock);
+{
-        try_join_result = joinee.try_join([&]() {
+    // We need to hold our lock to avoid a race where try_join succeeds
-            if (!add_to_blocker_set(joinee.m_join_blocker_set))
+    // but the joinee is joining immediately
-                m_should_block = false;
+    SpinlockLocker lock(m_lock);
-        });
+    m_try_join_result = m_joinee->try_join([&]() {
-        m_join_error = try_join_result.is_error();
+        if (!add_to_blocker_set(m_joinee->m_join_blocker_set))
-        if (m_join_error)
             m_should_block = false;
-    }
+    });
+    m_join_error = m_try_join_result.is_error();
+    if (m_join_error)
+        m_should_block = false;
+    return m_should_block;
 }
 
 void Thread::JoinBlocker::will_unblock_immediately_without_blocking(UnblockImmediatelyReason reason)
@@ -114,10 +123,16 @@ bool Thread::JoinBlocker::unblock(void* value, bool from_add_blocker)
 }
 
 Thread::WaitQueueBlocker::WaitQueueBlocker(WaitQueue& wait_queue, StringView block_reason)
-    : m_block_reason(block_reason)
+    : m_wait_queue(wait_queue)
+    , m_block_reason(block_reason)
 {
-    if (!add_to_blocker_set(wait_queue))
+}
+
+bool Thread::WaitQueueBlocker::setup_blocker()
+{
+    if (!add_to_blocker_set(m_wait_queue))
         m_should_block = false;
+    return m_should_block;
 }
 
 Thread::WaitQueueBlocker::~WaitQueueBlocker()
@@ -138,10 +153,16 @@ bool Thread::WaitQueueBlocker::unblock()
 }
 
 Thread::FutexBlocker::FutexBlocker(FutexQueue& futex_queue, u32 bitset)
-    : m_bitset(bitset)
+    : m_futex_queue(futex_queue)
+    , m_bitset(bitset)
 {
-    if (!add_to_blocker_set(futex_queue))
+}
+
+bool Thread::FutexBlocker::setup_blocker()
+{
+    if (!add_to_blocker_set(m_futex_queue))
         m_should_block = false;
+    return m_should_block;
 }
 
 Thread::FutexBlocker::~FutexBlocker()
@@ -187,10 +208,15 @@ Thread::FileDescriptionBlocker::FileDescriptionBlocker(FileDescription& descript
     : m_blocked_description(description)
     , m_flags(flags)
     , m_unblocked_flags(unblocked_flags)
+{
+}
+
+bool Thread::FileDescriptionBlocker::setup_blocker()
 {
     m_unblocked_flags = BlockFlags::None;
-    if (!add_to_blocker_set(description.blocker_set()))
+    if (!add_to_blocker_set(m_blocked_description->blocker_set()))
         m_should_block = false;
+    return m_should_block;
 }
 
 bool Thread::FileDescriptionBlocker::unblock(bool from_add_blocker, void*)
@@ -337,6 +363,10 @@ Thread::BlockResult Thread::SleepBlocker::block_result()
 
 Thread::SelectBlocker::SelectBlocker(FDVector& fds)
     : m_fds(fds)
+{
+}
+
+bool Thread::SelectBlocker::setup_blocker()
 {
     for (auto& fd_entry : m_fds) {
         fd_entry.unblocked_flags = FileBlocker::BlockFlags::None;
@@ -346,6 +376,7 @@ Thread::SelectBlocker::SelectBlocker(FDVector& fds)
         if (!fd_entry.description->blocker_set().add_blocker(*this, &fd_entry))
             m_should_block = false;
     }
+    return m_should_block;
 }
 
 Thread::SelectBlocker::~SelectBlocker()
@@ -590,7 +621,11 @@ Thread::WaitBlocker::WaitBlocker(int wait_options, idtype_t id_type, pid_t id, K
     , m_result(result)
     , m_should_block(!(m_wait_options & WNOHANG))
 {
-    switch (id_type) {
+}
+
+bool Thread::WaitBlocker::setup_blocker()
+{
+    switch (m_id_type) {
     case P_PID: {
         m_waitee = Process::from_pid(m_waitee_id);
         if (!m_waitee || m_waitee->ppid() != Process::current().pid()) {
@@ -613,11 +648,16 @@ Thread::WaitBlocker::WaitBlocker(int wait_options, idtype_t id_type, pid_t id, K
         VERIFY_NOT_REACHED();
     }
 
+    if (m_error)
+        return false;
+
+    if (m_wait_options & WNOHANG)
+        return false;
+
     // NOTE: unblock may be called within add_to_blocker_set, in which
     // case it means that we already have a match without having to block.
     // In that case add_to_blocker_set will return false.
-    if (m_error || !add_to_blocker_set(Process::current().wait_blocker_set()))
+    return add_to_blocker_set(Process::current().wait_blocker_set());
-        m_should_block = false;
 }
 
 void Thread::WaitBlocker::will_unblock_immediately_without_blocking(UnblockImmediatelyReason reason)