Remove unused Semaphore and fix pending_tasks publish ordering

Delete the unused Semaphore primitive and its test rather than fixing its
multi-waiter wakeup bug, since it is dead code referenced only by its own test.

Increment pending_tasks before publishing the task in Pool::submit and in the
get_task re-enqueue path, so a worker can never dequeue a task before its count
is registered (over-counting is safe; under-counting could transiently underflow).

Reword the ~Scheduler comments to describe the real failure mode: a dangling
current_pool leads to shared_from_this() on a destroyed pool (undefined
behaviour), not a guaranteed bad_weak_ptr throw.

Co-authored-by: Cursor <cursoragent@cursor.com>

Author: TrentHouliston

src/threading/scheduler/Pool.cpp

@@ -142,8 +142,8 @@ namespace threading {
             }
 
             const std::size_t bucket = queue::priority_index(task.task->priority);
-            buckets[bucket]->enqueue(std::move(task));
             pending_tasks.fetch_add(1, std::memory_order_release);
+            buckets[bucket]->enqueue(std::move(task));
 
             const std::lock_guard<std::mutex> lock(mutex);
             if (clear_idle) {
@@ -294,8 +294,8 @@ namespace threading {
                         // The task was dequeued but its lock isn't acquirable. Re-enqueue and
                         // wait for someone to notify us when the lock state changes.
                         const std::size_t bucket = queue::priority_index(task.task->priority);
-                        buckets[bucket]->enqueue(std::move(task));
                         pending_tasks.fetch_add(1, std::memory_order_release);
+                        buckets[bucket]->enqueue(std::move(task));
                     }
                 }
                 live = false;

src/threading/scheduler/Scheduler.cpp

@@ -50,10 +50,10 @@ namespace threading {
 
         Scheduler::~Scheduler() {
             // The constructor installed a non-owning pointer to the main thread pool in this thread's
-            // Pool::current_pool. Our pools are about to be destroyed, so leave no dangling pointer behind
-            // for any later Pool::current() call on this thread (it would otherwise throw bad_weak_ptr from
-            // the expired pool). Only clear it if it still refers to one of our pools, so we never disturb
-            // an unrelated Scheduler that may share this thread.
+            // Pool::current_pool. Our pools are about to be destroyed, so leave no dangling pointer behind for
+            // any later Pool::current() call on this thread (shared_from_this() on a destroyed pool is undefined
+            // behaviour, in practice observed as a bad_weak_ptr or a crash). Only clear it if it still refers to
+            // one of our pools, so we never disturb an unrelated Scheduler that may share this thread.
             const std::lock_guard<std::mutex> lock(pools_mutex);
             for (const auto& pool : pools) {
                 if (Pool::current_pool == pool.second.get()) {

src/threading/scheduler/Scheduler.hpp

@@ -53,9 +53,10 @@ namespace threading {
              *
              * The constructor points the creating thread's Pool::current_pool at the main thread pool so
              * work done before startup is attributed correctly. That pointer is non-owning, so once this
-             * Scheduler (and therefore its pools) is destroyed it would dangle; any later ReactionTask
-             * built on the same thread calls Pool::current() and would trip a bad_weak_ptr. Resetting it
-             * here keeps the pointer's lifetime bounded by the Scheduler that set it.
+             * Scheduler (and therefore its pools) is destroyed it would dangle; a later Pool::current()
+             * would call shared_from_this() on a destroyed pool, which is undefined behaviour (in practice
+             * observed as a bad_weak_ptr or a crash). Resetting it here keeps the pointer's lifetime bounded
+             * by the Scheduler that set it.
              */
             ~Scheduler();