diff --git a/kernel/arch/aarch64/sched/sched.cpp b/kernel/arch/aarch64/sched/sched.cpp index 8dc1c4cb..5ec3a610 100644 --- a/kernel/arch/aarch64/sched/sched.cpp +++ b/kernel/arch/aarch64/sched/sched.cpp @@ -191,7 +191,10 @@ __PRIVILEGED_CODE void on_yield(aarch64::trap_frame* tf) { save_cpu_context(tf, &prev->exec.cpu_ctx); prev->exec.tls_base = cpu::read_tls_base(); - if (!(prev->exec.flags & TASK_FLAG_KERNEL) && prev->state.load_relaxed() != TASK_STATE_DEAD) { + // A task inside a syscall still owns kernel state such as a linked wait + // node, so it dies at the syscall exit fatal check instead of here + if (!(prev->exec.flags & (TASK_FLAG_KERNEL | TASK_FLAG_IN_SYSCALL)) && + prev->state.load_relaxed() != TASK_STATE_DEAD) { uint32_t fsig = signals::fatal_pending(prev); if (fsig) { signals::die_from_signal(fsig); @@ -243,7 +246,10 @@ __PRIVILEGED_CODE void on_tick(aarch64::trap_frame* tf) { save_cpu_context(tf, &prev->exec.cpu_ctx); prev->exec.tls_base = cpu::read_tls_base(); - if (!(prev->exec.flags & TASK_FLAG_KERNEL) && prev->state.load_relaxed() != TASK_STATE_DEAD) { + // A task inside a syscall still owns kernel state such as a linked wait + // node, so it dies at the syscall exit fatal check instead of here + if (!(prev->exec.flags & (TASK_FLAG_KERNEL | TASK_FLAG_IN_SYSCALL)) && + prev->state.load_relaxed() != TASK_STATE_DEAD) { uint32_t fsig = signals::fatal_pending(prev); if (fsig) { signals::die_from_signal(fsig); diff --git a/kernel/arch/aarch64/timer/timer.cpp b/kernel/arch/aarch64/timer/timer.cpp index a7c14762..0843a28e 100644 --- a/kernel/arch/aarch64/timer/timer.cpp +++ b/kernel/arch/aarch64/timer/timer.cpp @@ -157,6 +157,9 @@ __PRIVILEGED_CODE bool on_interrupt() { return true; } + // Waking raw pointers under the queue lock satisfies the wake pin + // contract: a sleeper re-takes this lock in cancel_sleep before it + // can exit, and it slept on this CPU, so wake never spins off-CPU while (!state.sleep_queue.empty()) { sched::task* t = state.sleep_queue.front(); if (t->timer_deadline > now) break; diff --git a/kernel/arch/x86_64/sched/sched.cpp b/kernel/arch/x86_64/sched/sched.cpp index 3c17b070..f4aac94e 100644 --- a/kernel/arch/x86_64/sched/sched.cpp +++ b/kernel/arch/x86_64/sched/sched.cpp @@ -179,7 +179,10 @@ __PRIVILEGED_CODE void on_yield(x86::trap_frame* tf) { save_cpu_context(tf, &prev->exec.cpu_ctx); prev->exec.tls_base = cpu::read_tls_base(); - if (!(prev->exec.flags & TASK_FLAG_KERNEL) && prev->state.load_relaxed() != TASK_STATE_DEAD) { + // A task inside a syscall still owns kernel state such as a linked wait + // node, so it dies at the syscall exit fatal check instead of here + if (!(prev->exec.flags & (TASK_FLAG_KERNEL | TASK_FLAG_IN_SYSCALL)) && + prev->state.load_relaxed() != TASK_STATE_DEAD) { uint32_t fsig = signals::fatal_pending(prev); if (fsig) { signals::die_from_signal(fsig); @@ -230,7 +233,10 @@ __PRIVILEGED_CODE void on_tick(x86::trap_frame* tf) { save_cpu_context(tf, &prev->exec.cpu_ctx); prev->exec.tls_base = cpu::read_tls_base(); - if (!(prev->exec.flags & TASK_FLAG_KERNEL) && prev->state.load_relaxed() != TASK_STATE_DEAD) { + // A task inside a syscall still owns kernel state such as a linked wait + // node, so it dies at the syscall exit fatal check instead of here + if (!(prev->exec.flags & (TASK_FLAG_KERNEL | TASK_FLAG_IN_SYSCALL)) && + prev->state.load_relaxed() != TASK_STATE_DEAD) { uint32_t fsig = signals::fatal_pending(prev); if (fsig) { signals::die_from_signal(fsig); diff --git a/kernel/arch/x86_64/timer/timer.cpp b/kernel/arch/x86_64/timer/timer.cpp index 2615ad71..0414e7a2 100644 --- a/kernel/arch/x86_64/timer/timer.cpp +++ b/kernel/arch/x86_64/timer/timer.cpp @@ -223,6 +223,9 @@ __PRIVILEGED_CODE bool on_interrupt() { return true; } + // Waking raw pointers under the queue lock satisfies the wake pin + // contract: a sleeper re-takes this lock in cancel_sleep before it + // can exit, and it slept on this CPU, so wake never spins off-CPU while (!state.sleep_queue.empty()) { sched::task* t = state.sleep_queue.front(); if (t->timer_deadline > now) break; diff --git a/kernel/rc/strong_ref.h b/kernel/rc/strong_ref.h index 7222401c..15be76e8 100644 --- a/kernel/rc/strong_ref.h +++ b/kernel/rc/strong_ref.h @@ -79,8 +79,8 @@ class strong_ref { } /** - * Wrap a raw pointer whose refcount is already 1 (from allocation). - * Does NOT call add_ref. + * Wrap a raw pointer, taking ownership of one already-held reference, + * as when adopting a freshly allocated object. Does NOT call add_ref. */ [[nodiscard]] static strong_ref adopt(T* raw) noexcept { return strong_ref(raw, ADOPT_REF); diff --git a/kernel/resource/providers/proc_provider.cpp b/kernel/resource/providers/proc_provider.cpp index d4f667c9..52823f6c 100644 --- a/kernel/resource/providers/proc_provider.cpp +++ b/kernel/resource/providers/proc_provider.cpp @@ -1,12 +1,8 @@ #include "resource/providers/proc_provider.h" #include "sched/sched.h" #include "sched/task.h" -#include "sched/task_registry.h" -#include "mm/mm.h" #include "mm/vma.h" -#include "mm/vmm.h" #include "mm/heap.h" -#include "fs/node.h" #include "common/logging.h" #include "sync/poll.h" #include "sync/wait_queue.h" @@ -39,24 +35,29 @@ __PRIVILEGED_CODE static void proc_close(resource_object* obj) { auto* impl = static_cast(obj->impl); auto* pr = impl->proc.ptr(); + // The reference taken under pr->lock keeps the child reclaim-safe + // after the lock drops, even if it exits and is reaped concurrently sync::irq_state irq = sync::spin_lock_irqsave(pr->lock); + rc::strong_ref child; + bool unstarted = false; - if (pr->child && pr->child->state.load_relaxed() == sched::TASK_STATE_CREATED) { - auto* child = pr->child; - pr->child = nullptr; - sync::spin_unlock_irqrestore(pr->lock, irq); + if (pr->child) { + unstarted = pr->child->state.load_relaxed() == sched::TASK_STATE_CREATED; + if (unstarted || (!pr->exited && !pr->detached)) { + child = sched::task_ref(pr->child); + } - if (child->proc_res) { - (void)child->proc_res->release(); - child->proc_res = nullptr; + if (unstarted) { + pr->child = nullptr; } - destroy_unstarted_task(child); - } else if (pr->child && !pr->exited && !pr->detached) { - sched::task* child = pr->child; - sync::spin_unlock_irqrestore(pr->lock, irq); - sched::force_wake_for_kill(child); - } else { - sync::spin_unlock_irqrestore(pr->lock, irq); + } + + sync::spin_unlock_irqrestore(pr->lock, irq); + + if (child && unstarted) { + destroy_unstarted_task(child.ptr()); + } else if (child) { + sched::force_wake_for_kill(child.ptr()); } heap::kfree_delete(impl); @@ -164,44 +165,37 @@ __PRIVILEGED_CODE proc_resource* get_proc_resource(resource_object* obj) { __PRIVILEGED_CODE void destroy_unstarted_task(sched::task* t) { // Claim the task, a concurrent group teardown may have already - // moved it to dead and handed the memory to the reaper + // moved it to dead and handed it to the reaper uint32_t expected = sched::TASK_STATE_CREATED; if (!t->state.cmpxchg_strong_acq_rel(expected, sched::TASK_STATE_DEAD)) { return; } - // Leave the registry before the group teardown so registry walkers - // never see a task whose group is being freed (same order as reap_task) - sched::g_task_registry.remove(*t); - - resource::release_task_handles(t); - if (t->cwd) { - if (t->cwd->release()) { - fs::node::ref_destroy(t->cwd); + // Winning the claim confers sole ownership of the task's proc + // resource reference, a losing teardown path must not touch it + if (t->proc_res) { + if (t->proc_res->release()) { + proc_resource::ref_destroy(t->proc_res); } - t->cwd = nullptr; + t->proc_res = nullptr; } - if (t->exec.mm_ctx) { - mm::mm_context_release(t->exec.mm_ctx); - t->exec.mm_ctx = nullptr; + // Unlink from the group list here, reap_task releases the group + // reference but never touches the thread list + if (t->group && t->group->leader != t && t->group_link.is_linked()) { + sync::irq_state irq = sync::spin_lock_irqsave(t->group->lock); + t->group->threads.remove(t); + t->group->thread_count--; + sync::spin_unlock_irqrestore(t->group->lock, irq); } - if (t->group) { - if (t->group->leader != t && t->group_link.is_linked()) { - sync::irq_state irq = sync::spin_lock_irqsave(t->group->lock); - t->group->threads.remove(t); - t->group->thread_count--; - sync::spin_unlock_irqrestore(t->group->lock, irq); - } - if (t->group->release()) { - sched::thread_group::ref_destroy(t->group); - } - t->group = nullptr; - } + // Never started means already off-CPU, so the task goes straight to + // the reaper for the same staged teardown as a normal exit + t->cleanup_stage.store_release(sched::TASK_CLEANUP_STAGE_SCHEDULER_DETACHED); - vmm::free(t->sys_stack_base); - heap::kfree_delete(t); + if (t->release()) { + sched::task::ref_destroy(t); + } } } // namespace resource::proc_provider diff --git a/kernel/resource/providers/proc_provider.h b/kernel/resource/providers/proc_provider.h index 440592a5..34c7e562 100644 --- a/kernel/resource/providers/proc_provider.h +++ b/kernel/resource/providers/proc_provider.h @@ -47,7 +47,8 @@ __PRIVILEGED_CODE int32_t create_proc_resource( /** * @brief Destroy a task that was created but never started (TASK_STATE_CREATED). - * Frees mm_ctx, system stack, and the task struct. Does NOT release proc_res ref. + * Claims the task against concurrent group teardown, drops its proc + * resource reference, and defers reclamation to the reaper. * @note Privilege: **required** */ __PRIVILEGED_CODE void destroy_unstarted_task(sched::task* t); diff --git a/kernel/sched/sched.cpp b/kernel/sched/sched.cpp index a5231984..2e33d3e2 100644 --- a/kernel/sched/sched.cpp +++ b/kernel/sched/sched.cpp @@ -52,6 +52,10 @@ __PRIVILEGED_CODE void thread_group::ref_destroy(thread_group* self) { heap::kfree_delete(self); } +__PRIVILEGED_CODE void task::ref_destroy(task* self) { + rc::reaper::defer(&self->reaper_node); +} + constexpr size_t TASK_STACK_PAGES = 4; constexpr uint16_t TASK_GUARD_PAGES = 1; @@ -60,6 +64,8 @@ constexpr uint16_t SYSTEM_GUARD_PAGES = 1; constexpr uint64_t TLB_SYNC_CPU_IGNORED = ~0ULL; +constexpr uint32_t TEARDOWN_BATCH_SIZE = 16; + constexpr uint64_t AT_NULL = 0; constexpr uint64_t AT_PHDR = 3; constexpr uint64_t AT_PHENT = 4; @@ -98,6 +104,9 @@ __PRIVILEGED_CODE static inline void assert_switch_privilege_state( #endif /** + * Runs only after the last counted reference dropped. Registry lookups can + * still find the task until the removal below, its poisoned refcount turns + * them away. The TLB sync wait covers the freed stack pages, nothing else. * @note Privilege: **required** */ __PRIVILEGED_CODE static rc::reaper::cleanup_result reap_task(sched::task* t) { @@ -284,8 +293,11 @@ __PRIVILEGED_CODE void finalize_pending_off_cpu() { cpu::send_event(); if (load_cleanup_stage(pending) == TASK_CLEANUP_STAGE_SCHEDULER_DETACHED) { - // The reaper must only start cleanup after off-CPU publication is visible. - rc::reaper::defer(&pending->reaper_node); + // Reclamation must not begin before the off-CPU store above is + // visible, so the reference the task was created with drops here. + if (pending->release()) { + task::ref_destroy(pending); + } } } @@ -420,6 +432,24 @@ __PRIVILEGED_CODE void enqueue_on(task* t, uint32_t cpu_id) { sync::spin_unlock_irqrestore(rq.lock, irq); } +/** + * @note Privilege: **required** + */ +__PRIVILEGED_CODE rc::strong_ref task_ref(task* t) { + return rc::strong_ref::try_from_raw(t); +} + +/** + * @note Privilege: **required** + */ +__PRIVILEGED_CODE rc::strong_ref task_ref_by_tid(uint32_t tid) { + sync::irq_state irq = g_task_registry.lock(); + rc::strong_ref ref = task_ref(g_task_registry.find_locked(tid)); + g_task_registry.unlock(irq); + + return ref; +} + /** * @note Privilege: **required** */ @@ -519,43 +549,84 @@ __PRIVILEGED_CODE void sleep_ms(uint64_t ms) { reap_status = static_cast(es) & 0x7F; } - sync::irq_state irq = sync::spin_lock_irqsave(tg->lock); - auto it = tg->threads.begin(); - auto end = tg->threads.end(); - - while (it != end) { - sched::task& thread = *it; - ++it; // advance before potential removal - uint32_t expected = TASK_STATE_CREATED; - if (thread.state.cmpxchg_strong_acq_rel(expected, - TASK_STATE_DEAD)) { - tg->threads.remove(&thread); - tg->thread_count--; - if (thread.proc_res) { - auto* pr = thread.proc_res; - - sync::irq_state pr_irq = sync::spin_lock_irqsave(pr->lock); - pr->wait_status = reap_status; - pr->exited = true; - pr->child = nullptr; - sync::wake_all(pr->wait_queue); - sync::spin_unlock_irqrestore(pr->lock, pr_irq); - - thread.proc_res = nullptr; - if (pr->release()) { - resource::proc_provider::proc_resource::ref_destroy(pr); + // Threads are handled in batches: kill references and orphaned + // proc resources collected under tg->lock are woken only after + // it drops, keeping the off-CPU spin in wake outside the lock + for (;;) { + rc::strong_ref kill_batch[TEARDOWN_BATCH_SIZE]; + rc::strong_ref pr_batch[TEARDOWN_BATCH_SIZE]; + uint32_t kills = 0; + uint32_t prs = 0; + bool rescan = false; + + sync::irq_state irq = sync::spin_lock_irqsave(tg->lock); + auto it = tg->threads.begin(); + auto end = tg->threads.end(); + + while (it != end) { + sched::task& thread = *it; + ++it; // advance before potential removal + + if (kills == TEARDOWN_BATCH_SIZE || prs == TEARDOWN_BATCH_SIZE) { + rescan = true; + break; + } + + uint32_t expected = TASK_STATE_CREATED; + if (thread.state.cmpxchg_strong_acq_rel(expected, + TASK_STATE_DEAD)) { + tg->threads.remove(&thread); + tg->thread_count--; + + if (thread.proc_res) { + auto* pr = thread.proc_res; + + sync::irq_state pr_irq = sync::spin_lock_irqsave(pr->lock); + pr->wait_status = reap_status; + pr->exited = true; + pr->child = nullptr; + sync::spin_unlock_irqrestore(pr->lock, pr_irq); + + // The thread's resource reference moves to the + // batch and is released after the deferred wake + thread.proc_res = nullptr; + pr_batch[prs++] = rc::strong_ref::adopt(pr); } + + store_cleanup_stage(&thread, TASK_CLEANUP_STAGE_SCHEDULER_DETACHED); + if (thread.release()) { + task::ref_destroy(&thread); + } + } else if (!(thread.sig.pending.load_acquire() + & signals::sig_bit(signals::SIGKILL))) { + // Setting the kill bit under tg->lock marks the + // thread handled, so a rescan cannot batch it twice + thread.sig.pending.fetch_or_acq_rel( + signals::sig_bit(signals::SIGKILL)); + + kill_batch[kills++] = task_ref(&thread); } + } - store_cleanup_stage(&thread, TASK_CLEANUP_STAGE_SCHEDULER_DETACHED); - rc::reaper::defer(&thread.reaper_node); - } else { - force_wake_for_kill(&thread); + if (!rescan) { + tg->leader = nullptr; } - } + sync::spin_unlock_irqrestore(tg->lock, irq); - tg->leader = nullptr; - sync::spin_unlock_irqrestore(tg->lock, irq); + for (uint32_t i = 0; i < prs; i++) { + sync::wake_all(pr_batch[i]->wait_queue); + } + + for (uint32_t i = 0; i < kills; i++) { + if (kill_batch[i]) { + force_wake_for_kill(kill_batch[i].ptr()); + } + } + + if (!rescan) { + break; + } + } } else if (task->group_link.is_linked()) { sync::irq_state irq = sync::spin_lock_irqsave(tg->lock); tg->threads.remove(task); @@ -570,6 +641,7 @@ __PRIVILEGED_CODE void sleep_ms(uint64_t ms) { uint32_t ges = task->group ? task->group->group_exit_status.load_acquire() : 0; + bool announce = false; sync::irq_state irq = sync::spin_lock_irqsave(pr->lock); if (!pr->detached) { @@ -584,12 +656,18 @@ __PRIVILEGED_CODE void sleep_ms(uint64_t ms) { } pr->exited = true; pr->child = nullptr; - sync::wake_all(pr->wait_queue); + announce = true; } else { pr->child = nullptr; } sync::spin_unlock_irqrestore(pr->lock, irq); + // Waiters recheck pr->exited under pr->lock, so waking after + // the lock drops cannot lose the wakeup + if (announce) { + sync::wake_all(pr->wait_queue); + } + task->proc_res = nullptr; if (pr->release()) { resource::proc_provider::proc_resource::ref_destroy(pr); @@ -1301,11 +1379,6 @@ __PRIVILEGED_CODE int32_t init_ap(uint32_t cpu_id, uintptr_t task_stack_top, return ERR_NO_MEM; } - auto* dst = reinterpret_cast(idle); - for (size_t i = 0; i < sizeof(task); i++) { - dst[i] = 0; - } - idle->exec.flags = TASK_FLAG_IDLE | TASK_FLAG_ELEVATED | TASK_FLAG_KERNEL | TASK_FLAG_CAN_ELEVATE | TASK_FLAG_PREEMPTIBLE; idle->exec.cpu = cpu_id; diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h index b4529821..9ccd2049 100644 --- a/kernel/sched/sched.h +++ b/kernel/sched/sched.h @@ -2,6 +2,7 @@ #define STELLUX_SCHED_SCHED_H #include "common/types.h" +#include "rc/strong_ref.h" namespace exec { struct loaded_image; } @@ -140,9 +141,15 @@ __PRIVILEGED_CODE void enqueue(task* t); __PRIVILEGED_CODE void enqueue_on(task* t, uint32_t cpu_id); /** - * @brief Resume a blocked task by placing it on the local runqueue. + * @brief Resume a blocked task by placing it on its CPU's runqueue. * Atomically transitions BLOCKED -> READY via CAS. * Called by sync::wake_one / sync::wake_all. + * + * The caller must pin t so the reaper cannot free it mid-call: hold a + * counted reference (task_ref) or a lock t must take before it can exit. + * A remote wake spins until t leaves its CPU, so never hold a spinlock + * with interrupts off across the call. Waking a task that last ran on + * the calling CPU never spins, so the timer expiry walk is exempt. * @note Privilege: **required** */ __PRIVILEGED_CODE void wake(task* t); @@ -151,10 +158,29 @@ __PRIVILEGED_CODE void wake(task* t); * @brief Mark a task for termination and wake it if blocked. * Fire-and-forget: the target is force-woken now or observes the kill * at its next killable blocking attempt (sleep, futex, poll). + * Same pin and spin rules as wake. * @note Privilege: **required** */ __PRIVILEGED_CODE void force_wake_for_kill(task* t); +/** + * @brief Acquire a counted reference to a task from a raw pointer. + * The raw pointer must still be protected here: hold a lock the task must + * take before it can finish exiting, or another counted reference. Returns + * a null reference if the task is already tearing down. + * @note Privilege: **required** + */ +[[nodiscard]] __PRIVILEGED_CODE rc::strong_ref task_ref(task* t); + +/** + * @brief Acquire a counted reference to the task with the given tid. + * Takes the registry lock internally, so no caller-side pin is needed. + * Returns a null reference if no task with that tid is registered or if + * the task is already tearing down. + * @note Privilege: **required** + */ +[[nodiscard]] __PRIVILEGED_CODE rc::strong_ref task_ref_by_tid(uint32_t tid); + /** * @brief Publish intent to block: moves the current task to BLOCKED. * Pair with block_task_interrupted before yielding. diff --git a/kernel/sched/sched_internal.h b/kernel/sched/sched_internal.h index ffaa5596..2d64b91b 100644 --- a/kernel/sched/sched_internal.h +++ b/kernel/sched/sched_internal.h @@ -66,7 +66,8 @@ __PRIVILEGED_CODE void defer_off_cpu_finalize(task* prev); /** * Common: advances this CPU's TLB sync epoch. - * This marks a safe point that reaper can rely on before reclaiming stack pages. + * This marks a safe point that reaper can rely on before reclaiming stack + * pages. It says nothing about task pointers, which counted references pin. * @note Privilege: **required** */ __PRIVILEGED_CODE void advance_cpu_tlb_sync_epoch(); diff --git a/kernel/sched/task.h b/kernel/sched/task.h index 33bc675e..0200c527 100644 --- a/kernel/sched/task.h +++ b/kernel/sched/task.h @@ -18,6 +18,22 @@ namespace sched { constexpr size_t TASK_NAME_MAX = 256; +/** + * Task states and the legal transitions between them: + * + * CREATED -> READY enqueue / enqueue_on (CAS) + * CREATED -> DEAD unstarted task teardown claims it (CAS) + * READY -> RUNNING pick_next_and_switch + * RUNNING -> READY preemption re-enqueue + * RUNNING -> BLOCKED prepare_to_block_task, by the task itself + * RUNNING -> DEAD exit, by the task itself + * BLOCKED -> READY wake (CAS) + * BLOCKED -> RUNNING cancel_block_task, by the task itself (CAS) + * + * A task is BLOCKED yet still on-CPU between prepare_to_block_task and + * its yield. A tick in that window switches it out without re-enqueueing + * it, so it stays off the runqueues until a wake arrives. + */ constexpr uint32_t TASK_STATE_CREATED = 0; // exists but not on any queue constexpr uint32_t TASK_STATE_READY = 1; // on a runqueue constexpr uint32_t TASK_STATE_RUNNING = 2; // executing on a CPU @@ -26,6 +42,14 @@ constexpr uint32_t TASK_STATE_DEAD = 4; // terminated constexpr int32_t TASK_KILL_STATUS = 9; // wait-status for forcibly killed tasks +/** + * Reclamation ladder. exit() records EXIT_REQUESTED on the dying task, the + * scheduler advances to SCHEDULER_DETACHED when the task is switched out + * (unstarted teardown jumps there directly), and the reaper owns the last + * two stages: it snapshots every CPU's TLB sync epoch, waits for each CPU + * to move past its snapshot, then reclaims. The struct itself is freed only + * after the last counted reference has dropped and handed it to the reaper. + */ constexpr uint32_t TASK_CLEANUP_STAGE_ACTIVE = 0; constexpr uint32_t TASK_CLEANUP_STAGE_EXIT_REQUESTED = 1; constexpr uint32_t TASK_CLEANUP_STAGE_SCHEDULER_DETACHED = 2; @@ -36,7 +60,9 @@ constexpr uint32_t TASK_CLEANUP_STAGE_READY_TO_RECLAIM = 4; * Per-task TLB sync ticket used by reaper before reclaiming task stacks. * * The ticket snapshots each CPU's reclaim epoch and requires every CPU to - * advance past that snapshot before stack unmap/free can proceed. + * advance past that snapshot before stack unmap/free can proceed. It only + * retires stale TLB entries for the freed stacks, keeping the task struct + * itself alive is the job of its counted references. */ struct task_tlb_sync_ticket { uint64_t cpu_epoch_snapshot[MAX_CPUS]; @@ -45,7 +71,12 @@ struct task_tlb_sync_ticket { struct thread_group; -struct task { +/** + * A schedulable unit of execution. Refcounted: a task starts with one + * reference, dropped once the scheduler has detached it after death, and + * the last release hands reclamation to the reaper. + */ +struct task : rc::ref_counted { // Execution core task_exec_core exec; @@ -89,12 +120,14 @@ struct task { // when a thread is created with POSIX file table semantics resource::handle_table* handles; resource::proc_provider::proc_resource* proc_res; -}; -// Assembly accesses task_exec_core fields via offsets from the task pointer. -// exec must be at offset 0 so &task == &task.exec. -static_assert(__builtin_offsetof(task, exec) == 0, - "task.exec must be at offset 0 for assembly compatibility"); + /** + * Defers reclamation to the reaper, which owns the staged teardown + * and the TLB grace period for stack pages. + * @note Privilege: **required** + */ + __PRIVILEGED_CODE static void ref_destroy(task* self); +}; /** * Groups all tasks sharing an address space. Every userland task belongs to diff --git a/kernel/signals/signal.cpp b/kernel/signals/signal.cpp index 6987e5c8..3b8d1134 100644 --- a/kernel/signals/signal.cpp +++ b/kernel/signals/signal.cpp @@ -15,10 +15,6 @@ enum class send_verdict : uint8_t { HANDLED, // a user handler is installed, wake the target to deliver }; -// Distinct groups remembered during one group-id send. Signals coalesce, -// so duplicate sends past this window are harmless extra wakes. -constexpr uint32_t MAX_GROUP_SEND_GROUPS = 64; - /** * Clear pending instances of sig from the shared set and every thread. * @note Privilege: **required** @@ -173,13 +169,20 @@ __PRIVILEGED_CODE int32_t send_to_task(sched::task* t, uint32_t sig) { // at its next kernel crossing, not only after leader teardown sched::thread_group* tg = t->group; tg->sig.shared_pending.fetch_or_acq_rel(sig_bit(SIGKILL)); + rc::strong_ref leader; + sync::irq_state irq = sync::spin_lock_irqsave(tg->lock); if (tg->leader && tg->leader != t) { - sched::force_wake_for_kill(tg->leader); + leader = sched::task_ref(tg->leader); } sync::spin_unlock_irqrestore(tg->lock, irq); + + if (leader) { + sched::force_wake_for_kill(leader.ptr()); + } + sched::force_wake_for_kill(t); return OK; } @@ -209,13 +212,20 @@ __PRIVILEGED_CODE int32_t send_to_group(sched::thread_group* tg, uint32_t sig) { if (sig == SIGKILL) { tg->sig.shared_pending.fetch_or_acq_rel(sig_bit(SIGKILL)); + rc::strong_ref leader; + sync::irq_state irq = sync::spin_lock_irqsave(tg->lock); if (tg->leader) { - sched::force_wake_for_kill(tg->leader); // leader exit reaps the group + leader = sched::task_ref(tg->leader); } sync::spin_unlock_irqrestore(tg->lock, irq); + + if (leader) { + sched::force_wake_for_kill(leader.ptr()); // leader exit reaps the group + } + return OK; } @@ -248,56 +258,68 @@ __PRIVILEGED_CODE int32_t send_to_group(sched::thread_group* tg, uint32_t sig) { tg->sig.shared_pending.fetch_or_acq_rel(bit); + rc::strong_ref target; if (verdict != send_verdict::IGNORABLE) { // Wake one thread the signal can act on now, leader preferred bool needs_handler = verdict == send_verdict::HANDLED; - sched::task* target = nullptr; if (tg->leader && wake_eligible(tg->leader, bit, needs_handler)) { - target = tg->leader; + target = sched::task_ref(tg->leader); } else { for (sched::task& thread : tg->threads) { if (wake_eligible(&thread, bit, needs_handler)) { - target = &thread; + target = sched::task_ref(&thread); break; } } } - if (target) { - wake_for_signal(target); - } } sync::spin_unlock_irqrestore(tg->lock, irq); + + if (target) { + wake_for_signal(target.ptr()); + } + return OK; } __PRIVILEGED_CODE int32_t send_to_group_id(uint32_t group_id, uint32_t sig) { - sched::thread_group* seen[MAX_GROUP_SEND_GROUPS]; - uint32_t seen_count = 0; bool found = false; + uint32_t cursor = 0; - sync::irq_state irq = sched::g_task_registry.lock(); - sched::g_task_registry.for_each_locked([&](sched::task& t) { - sched::thread_group* tg = t.group; - if (!tg || tg->group_id.load_acquire() != group_id) { - return; - } + // Each pass sends to the matching group with the smallest leader pid + // above the cursor, reaching every group however many match + for (;;) { + sched::thread_group* best = nullptr; - for (uint32_t i = 0; i < seen_count; i++) { - if (seen[i] == tg) { + sync::irq_state irq = sched::g_task_registry.lock(); + sched::g_task_registry.for_each_locked([&](sched::task& t) { + sched::thread_group* tg = t.group; + if (!tg || tg->group_id.load_acquire() != group_id) { return; } - } - if (seen_count < MAX_GROUP_SEND_GROUPS) { - seen[seen_count++] = tg; + + if (tg->pid > cursor && (!best || tg->pid < best->pid)) { + best = tg; + } + }); + + // A registry member pins its group until reaped, so the reference + // is taken under the registry lock and the send runs after it drops + rc::strong_ref target = + rc::strong_ref::try_from_raw(best); + sched::g_task_registry.unlock(irq); + + if (!target) { + break; } found = true; + cursor = target->pid; if (sig != 0) { - send_to_group(tg, sig); + send_to_group(target.ptr(), sig); } - }); - sched::g_task_registry.unlock(irq); + } return found ? OK : ERR_INVAL; } @@ -497,11 +519,16 @@ __PRIVILEGED_CODE void die_from_signal(uint32_t sig) { // A dying non-leader force-kills the leader, whose exit reaps // every remaining thread + rc::strong_ref leader; sync::irq_state irq = sync::spin_lock_irqsave(tg->lock); if (tg->leader && tg->leader != self) { - sched::force_wake_for_kill(tg->leader); + leader = sched::task_ref(tg->leader); } sync::spin_unlock_irqrestore(tg->lock, irq); + + if (leader) { + sched::force_wake_for_kill(leader.ptr()); + } } // The SIGKILL bit makes exit() encode a killed-by-signal wait status diff --git a/kernel/sync/futex.cpp b/kernel/sync/futex.cpp index 4354cb66..84572c84 100644 --- a/kernel/sync/futex.cpp +++ b/kernel/sync/futex.cpp @@ -128,7 +128,7 @@ __PRIVILEGED_CODE int32_t futex_wake(uintptr_t uaddr, uint32_t count) { uint32_t total_woken = 0; for (;;) { - sched::task* batch[WAKE_BATCH_SIZE]; + rc::strong_ref batch[WAKE_BATCH_SIZE]; uint32_t n = 0; bool done = false; @@ -141,7 +141,7 @@ __PRIVILEGED_CODE int32_t futex_wake(uintptr_t uaddr, uint32_t count) { ++it; // advance before removal if (w.mm == mm && w.addr == uaddr) { bucket->waiters.remove(&w); - batch[n++] = w.task; + batch[n++] = sched::task_ref(w.task); if (total_woken + n >= count) { done = true; break; @@ -153,7 +153,9 @@ __PRIVILEGED_CODE int32_t futex_wake(uintptr_t uaddr, uint32_t count) { spin_unlock_irqrestore(bucket->lock, irq); for (uint32_t i = 0; i < n; i++) { - sched::wake(batch[i]); + if (batch[i]) { + sched::wake(batch[i].ptr()); + } } total_woken += n; @@ -174,7 +176,7 @@ __PRIVILEGED_CODE int32_t futex_wake_all(uintptr_t uaddr) { uint32_t total_woken = 0; for (;;) { - sched::task* batch[WAKE_BATCH_SIZE]; + rc::strong_ref batch[WAKE_BATCH_SIZE]; uint32_t n = 0; irq_state irq = spin_lock_irqsave(bucket->lock); @@ -186,7 +188,7 @@ __PRIVILEGED_CODE int32_t futex_wake_all(uintptr_t uaddr) { ++it; if (w.mm == mm && w.addr == uaddr) { bucket->waiters.remove(&w); - batch[n++] = w.task; + batch[n++] = sched::task_ref(w.task); } } @@ -194,7 +196,9 @@ __PRIVILEGED_CODE int32_t futex_wake_all(uintptr_t uaddr) { spin_unlock_irqrestore(bucket->lock, irq); for (uint32_t i = 0; i < n; i++) { - sched::wake(batch[i]); + if (batch[i]) { + sched::wake(batch[i].ptr()); + } } total_woken += n; diff --git a/kernel/sync/wait_queue.cpp b/kernel/sync/wait_queue.cpp index 152a51b3..18eb84d2 100644 --- a/kernel/sync/wait_queue.cpp +++ b/kernel/sync/wait_queue.cpp @@ -8,14 +8,11 @@ namespace sync { /** * Set triggered on all observers and wake their tasks. * - * Under wq.lock: set triggered on every observer and snapshot all task - * pointers into a stack batch. After releasing the lock, wake each task. - * - * The observer count per wait_queue is bounded in practice by the number - * of tasks simultaneously polling the same source (typically 1-2). - * The batch is sized to 16 which covers all realistic cases. If more - * observers exist, the excess are handled by a single-entry fallback - * that re-scans under the lock. + * Whoever flips an observer's triggered flag from 0 to 1 owes it exactly + * one wake, delivered after wq.lock drops so sched::wake's off-CPU spin + * never runs under the lock. Already-triggered observers are skipped, an + * earlier notify owes their wake. A full batch forces a rescan, the flag + * marks who was already handled. */ constexpr uint32_t OBSERVER_BATCH_SIZE = 16; constexpr uint32_t WAITER_BATCH_SIZE = 16; @@ -23,35 +20,38 @@ constexpr uint32_t WAITER_BATCH_SIZE = 16; __PRIVILEGED_CODE static void notify_observers_and_unlock( wait_queue& wq, irq_state irq ) { - sched::task* batch[OBSERVER_BATCH_SIZE]; - uint32_t n = 0; - uint32_t total = 0; - - for (auto& obs : wq.observers) { - obs.table->triggered.store_release(1); - if (n < OBSERVER_BATCH_SIZE) { - batch[n++] = obs.table->task; - } - total++; - } + for (;;) { + rc::strong_ref batch[OBSERVER_BATCH_SIZE]; + uint32_t n = 0; + bool rescan = false; - spin_unlock_irqrestore(wq.lock, irq); + for (auto& obs : wq.observers) { + if (obs.table->triggered.load_acquire()) { + continue; + } - for (uint32_t i = 0; i < n; i++) { - sched::wake(batch[i]); - } + if (n == OBSERVER_BATCH_SIZE) { + rescan = true; + break; + } - // Overflow: re-scan and wake all observers under the lock. - // Some were already woken above, sched::wake() is idempotent. - // sched::wake() only acquires rq.lock (never wq.lock), so holding - // wq.lock here is deadlock-free. This path is extremely rare - // (requires >16 concurrent pollers on one wait queue). - if (total > n) { - irq = spin_lock_irqsave(wq.lock); - for (auto& obs : wq.observers) { - sched::wake(obs.table->task); + obs.table->triggered.store_release(1); + batch[n++] = sched::task_ref(obs.table->task); } + spin_unlock_irqrestore(wq.lock, irq); + + for (uint32_t i = 0; i < n; i++) { + if (batch[i]) { + sched::wake(batch[i].ptr()); + } + } + + if (!rescan) { + return; + } + + irq = spin_lock_irqsave(wq.lock); } } @@ -93,7 +93,7 @@ irq_state wait(wait_queue& wq, spinlock& lock, irq_state saved) { */ __PRIVILEGED_CODE void wake_one(wait_queue& wq) { irq_state irq = spin_lock_irqsave(wq.lock); - sched::task* t = wq.waiters.pop_front(); + rc::strong_ref t = sched::task_ref(wq.waiters.pop_front()); if (!wq.observers.empty()) { // notify_observers_and_unlock releases wq.lock @@ -103,13 +103,13 @@ __PRIVILEGED_CODE void wake_one(wait_queue& wq) { } if (t) { - sched::wake(t); + sched::wake(t.ptr()); } } /** - * Wake all waiting tasks. Snapshots waiter pointers into a stack batch - * so wait_link is fully unlinked (prev=next=nullptr) before any task + * Wake all waiting tasks. Snapshots counted waiter references into a stack + * batch so wait_link is fully unlinked (prev=next=nullptr) before any task * can be scheduled. This prevents a concurrent force_wake_for_kill from * racing with post-yield cleanup in sync::wait, which assumes is_linked * means "still on wq.waiters". @@ -117,14 +117,13 @@ __PRIVILEGED_CODE void wake_one(wait_queue& wq) { * @note Privilege: **required** */ __PRIVILEGED_CODE void wake_all(wait_queue& wq) { - sched::task* batch[WAITER_BATCH_SIZE]; - for (;;) { + rc::strong_ref batch[WAITER_BATCH_SIZE]; uint32_t n = 0; irq_state irq = spin_lock_irqsave(wq.lock); while (!wq.waiters.empty() && n < WAITER_BATCH_SIZE) { - batch[n++] = wq.waiters.pop_front(); + batch[n++] = sched::task_ref(wq.waiters.pop_front()); } bool drained = wq.waiters.empty(); @@ -135,7 +134,9 @@ __PRIVILEGED_CODE void wake_all(wait_queue& wq) { } for (uint32_t i = 0; i < n; i++) { - sched::wake(batch[i]); + if (batch[i]) { + sched::wake(batch[i].ptr()); + } } if (drained) break; diff --git a/kernel/sync/wait_queue.h b/kernel/sync/wait_queue.h index ac5c25ec..19eaf57d 100644 --- a/kernel/sync/wait_queue.h +++ b/kernel/sync/wait_queue.h @@ -25,10 +25,8 @@ struct wait_queue { * Block current task until woken, atomically releasing a held lock. * * Caller MUST hold `lock` via spin_lock_irqsave (IRQs disabled). - * Internally acquires wq.lock via spin_lock_irqsave for both the - * enqueue and post-yield cleanup, ensuring it is - * safe to call from any context, including paths where an ISR may - * concurrently call wake_one() or wake_all(). + * Takes wq.lock for the enqueue and the post-yield cleanup, so an ISR + * running wake_one() or wake_all() cannot race the wait entry. * * On wake, re-acquires `lock` via spin_lock_irqsave and returns * the new irq_state. Caller MUST re-check its condition (spurious @@ -48,14 +46,16 @@ irq_state wait(wait_queue& wq, spinlock& lock, irq_state saved); /** * Wake the first waiting task (FIFO order). - * No-op if the queue is empty. Safe from IRQ context. + * No-op if the queue is empty. Waiters are pinned internally, but the + * off-CPU spin rule of sched::wake still applies to the caller. * @note Privilege: **required** */ __PRIVILEGED_CODE void wake_one(wait_queue& wq); /** * Wake all waiting tasks. - * No-op if the queue is empty. Safe from IRQ context. + * No-op if the queue is empty. Waiters are pinned internally, but the + * off-CPU spin rule of sched::wake still applies to the caller. * @note Privilege: **required** */ __PRIVILEGED_CODE void wake_all(wait_queue& wq); diff --git a/kernel/syscall/handlers/sys_proc.cpp b/kernel/syscall/handlers/sys_proc.cpp index 081245ab..ee9da6ae 100644 --- a/kernel/syscall/handlers/sys_proc.cpp +++ b/kernel/syscall/handlers/sys_proc.cpp @@ -486,17 +486,24 @@ DEFINE_SYSCALL1(proc_kill, u_handle) { return syscall::EINVAL; } + // The reference taken under pr->lock keeps the child reclaim-safe + // after the lock drops, even if it exits and is reaped concurrently sync::irq_state irq = sync::spin_lock_irqsave(pr->lock); - if (!pr->child || pr->exited) { - sync::spin_unlock_irqrestore(pr->lock, irq); - resource::resource_release(obj); - return pr->exited ? 0 : syscall::EINVAL; + bool exited = pr->exited; + + rc::strong_ref child; + if (pr->child && !exited) { + child = sched::task_ref(pr->child); } - sched::task* child = pr->child; sync::spin_unlock_irqrestore(pr->lock, irq); - RUN_ELEVATED(sched::force_wake_for_kill(child)); + if (!child) { + resource::resource_release(obj); + return exited ? 0 : syscall::EINVAL; + } + + RUN_ELEVATED(sched::force_wake_for_kill(child.ptr())); resource::resource_release(obj); return 0; diff --git a/kernel/syscall/handlers/sys_signal.cpp b/kernel/syscall/handlers/sys_signal.cpp index 0a23f7a6..e1c35935 100644 --- a/kernel/syscall/handlers/sys_signal.cpp +++ b/kernel/syscall/handlers/sys_signal.cpp @@ -28,24 +28,20 @@ static int64_t kill_process_group(uint32_t group_id, uint32_t sig) { // Thread-directed send shared by tkill and tgkill, tgid 0 skips the pair check static int64_t send_to_thread(uint32_t tgid, uint32_t tid, uint32_t sig) { - int64_t result = syscall::ESRCH; - - sync::irq_state irq = sched::g_task_registry.lock(); - sched::task* t = sched::g_task_registry.find_locked(tid); - if (t && (tgid == 0 || (t->group && t->group->pid == tgid))) { - if (sig != 0) { - result = map_send_error(signals::send_to_task(t, sig)); - } else { - // The null probe reports the same permission gate a send would - bool denied = (t->exec.flags & - (sched::TASK_FLAG_KERNEL | sched::TASK_FLAG_IDLE)) || - !t->group; - result = denied ? syscall::EPERM : 0; - } + rc::strong_ref t = sched::task_ref_by_tid(tid); + if (!t || (tgid != 0 && (!t->group || t->group->pid != tgid))) { + return syscall::ESRCH; + } + + if (sig == 0) { + // The null probe reports the same permission gate a send would + bool denied = (t->exec.flags & + (sched::TASK_FLAG_KERNEL | sched::TASK_FLAG_IDLE)) || + !t->group; + return denied ? syscall::EPERM : 0; } - sched::g_task_registry.unlock(irq); - return result; + return map_send_error(signals::send_to_task(t.ptr(), sig)); } DEFINE_SYSCALL4(rt_sigaction, signum, u_act, u_oldact, sigsetsize) { @@ -172,17 +168,13 @@ DEFINE_SYSCALL2(kill, u_pid, u_sig) { // Any thread id resolves to its containing process (kill semantics // on Linux), and the signal is delivered process-wide - int64_t result = syscall::ESRCH; - sync::irq_state irq = sched::g_task_registry.lock(); - sched::task* t = - sched::g_task_registry.find_locked(static_cast(pid)); - if (t && t->group) { - result = sig ? map_send_error(signals::send_to_group(t->group, sig)) - : 0; + rc::strong_ref t = + sched::task_ref_by_tid(static_cast(pid)); + if (!t || !t->group) { + return syscall::ESRCH; } - sched::g_task_registry.unlock(irq); - return result; + return sig ? map_send_error(signals::send_to_group(t->group, sig)) : 0; } // pid 0 targets the caller's group, below -1 the group named by -pid diff --git a/kernel/syscall/handlers/sys_task.cpp b/kernel/syscall/handlers/sys_task.cpp index 78caf59e..50ee6c2a 100644 --- a/kernel/syscall/handlers/sys_task.cpp +++ b/kernel/syscall/handlers/sys_task.cpp @@ -225,13 +225,19 @@ DEFINE_SYSCALL1(exit_group, status) { // A non leader forces the leader down, the leader's exit then // reaps every remaining thread including this one + rc::strong_ref leader; + sync::irq_state irq = sync::spin_lock_irqsave(tg->lock); if (tg->leader && tg->leader != self) { - sched::force_wake_for_kill(tg->leader); + leader = sched::task_ref(tg->leader); } sync::spin_unlock_irqrestore(tg->lock, irq); + + if (leader) { + sched::force_wake_for_kill(leader.ptr()); + } } sched::exit(static_cast(status)); diff --git a/kernel/tests/resource/resource.test.cpp b/kernel/tests/resource/resource.test.cpp index db4bf830..f7f00ce5 100644 --- a/kernel/tests/resource/resource.test.cpp +++ b/kernel/tests/resource/resource.test.cpp @@ -90,20 +90,23 @@ TEST(resource_test, rights_enforced_for_read_and_write) { TEST(resource_test, releasing_task_handles_invalidates_existing_handles) { // A scratch task exercises table teardown without touching the // test runner's own live handle table - sched::task scratch{}; - ASSERT_EQ(resource::init_task_handles(&scratch), resource::OK); + sched::task* scratch = heap::kalloc_new(); + ASSERT_NOT_NULL(scratch); + ASSERT_EQ(resource::init_task_handles(scratch), resource::OK); resource::handle_t h1 = -1; resource::handle_t h2 = -1; - ASSERT_EQ(resource::open(&scratch, "/resource_close_all_1", fs::O_CREAT | fs::O_RDWR, &h1), resource::OK); - ASSERT_EQ(resource::open(&scratch, "/resource_close_all_2", fs::O_CREAT | fs::O_RDWR, &h2), resource::OK); + ASSERT_EQ(resource::open(scratch, "/resource_close_all_1", fs::O_CREAT | fs::O_RDWR, &h1), resource::OK); + ASSERT_EQ(resource::open(scratch, "/resource_close_all_2", fs::O_CREAT | fs::O_RDWR, &h2), resource::OK); - resource::release_task_handles(&scratch); + resource::release_task_handles(scratch); - EXPECT_NULL(scratch.handles); - EXPECT_EQ(resource::close(&scratch, h1), resource::ERR_BADF); - EXPECT_EQ(resource::close(&scratch, h2), resource::ERR_BADF); + EXPECT_NULL(scratch->handles); + EXPECT_EQ(resource::close(scratch, h1), resource::ERR_BADF); + EXPECT_EQ(resource::close(scratch, h2), resource::ERR_BADF); + + heap::kfree_delete(scratch); } TEST(resource_test, used_handle_slots_never_have_unknown_type) { diff --git a/kernel/tests/sched/kill.test.cpp b/kernel/tests/sched/kill.test.cpp index e6a06634..d87419fb 100644 --- a/kernel/tests/sched/kill.test.cpp +++ b/kernel/tests/sched/kill.test.cpp @@ -56,9 +56,11 @@ TEST(kill, force_wake_kills_sleeping_task) { g_sleep_kill_elapsed_ns.store_relaxed(0); sched::task* t = nullptr; + rc::strong_ref pin; RUN_ELEVATED({ t = sched::create_kernel_task(sleep_kill_fn, nullptr, "kill_sleep"); ASSERT_NOT_NULL(t); + pin = sched::task_ref(t); sched::enqueue(t); }); @@ -133,9 +135,11 @@ TEST(kill, force_wake_kills_blocked_on_wq) { g_wq_kill_was_pending.store_relaxed(0); sched::task* t = nullptr; + rc::strong_ref pin; RUN_ELEVATED({ t = sched::create_kernel_task(wq_kill_fn, nullptr, "kill_wq"); ASSERT_NOT_NULL(t); + pin = sched::task_ref(t); sched::enqueue(t); }); @@ -178,9 +182,11 @@ TEST(kill, self_removal_cleans_wq) { g_sr_done.store_relaxed(0); sched::task* t = nullptr; + rc::strong_ref pin; RUN_ELEVATED({ t = sched::create_kernel_task(sr_waiter_fn, nullptr, "kill_sr"); ASSERT_NOT_NULL(t); + pin = sched::task_ref(t); sched::enqueue(t); }); @@ -226,9 +232,11 @@ TEST(kill, double_kill_is_harmless) { g_double_kp.store_relaxed(0); sched::task* t = nullptr; + rc::strong_ref pin; RUN_ELEVATED({ t = sched::create_kernel_task(double_kill_fn, nullptr, "kill_dbl"); ASSERT_NOT_NULL(t); + pin = sched::task_ref(t); sched::enqueue(t); }); diff --git a/kernel/tests/sched/task_registry.test.cpp b/kernel/tests/sched/task_registry.test.cpp index d2d95da8..c61ff53b 100644 --- a/kernel/tests/sched/task_registry.test.cpp +++ b/kernel/tests/sched/task_registry.test.cpp @@ -17,7 +17,7 @@ static sched::task s_tasks[MAX_MOCK_TASKS]; // Zero-initialize a mock task and set its TID. Only the tid and // task_registry_link fields matter for registry operations. static void init_mock_task(sched::task& t, uint32_t tid) { - new (&t) sched::task{}; + new (&t) sched::task(); t.tid = tid; } diff --git a/kernel/tests/signals/signal_send.test.cpp b/kernel/tests/signals/signal_send.test.cpp index 18f42b55..1c06ad53 100644 --- a/kernel/tests/signals/signal_send.test.cpp +++ b/kernel/tests/signals/signal_send.test.cpp @@ -3,6 +3,7 @@ #include "stlx_unit_test.h" #include "signals/signal.h" #include "sched/task.h" +#include "sched/task_registry.h" #include "mm/heap.h" #include "dynpriv/dynpriv.h" @@ -160,3 +161,73 @@ TEST(signal_send, stop_class_send_is_ignored) { EXPECT_EQ(rc, signals::OK); EXPECT_EQ(g_thread->sig.pending.load_relaxed(), 0ULL); } + +// A group-id send must reach every matching group, not only the ones +// that fit a fixed window. Sized past the 64-group batch an earlier +// implementation silently dropped. +constexpr uint32_t GID_GROUP_COUNT = 67; +constexpr uint32_t GID_TEST_GROUP = 0x7E577E57u; +constexpr uint32_t GID_TID_BASE = 0x40000000u; + +static sched::task* g_gid_leaders[GID_GROUP_COUNT]; +static sched::thread_group* g_gid_groups[GID_GROUP_COUNT]; + +TEST(signal_send, group_id_send_reaches_all_groups) { + bool built = true; + + // Single-task mock processes registered in the live task registry, + // all members of the same process group + RUN_ELEVATED({ + for (uint32_t i = 0; i < GID_GROUP_COUNT; i++) { + g_gid_leaders[i] = heap::kalloc_new(); + g_gid_groups[i] = heap::kalloc_new(); + if (!g_gid_leaders[i] || !g_gid_groups[i]) { + built = false; + break; + } + + sched::task* t = g_gid_leaders[i]; + sched::thread_group* tg = g_gid_groups[i]; + tg->lock = sync::SPINLOCK_INIT; + tg->leader = t; + tg->pid = GID_TID_BASE + i; + tg->threads.init(); + tg->group_id.store_relaxed(GID_TEST_GROUP); + + t->tid = GID_TID_BASE + i; + t->group = tg; + sched::g_task_registry.insert(t); + } + }); + + int32_t rc = -1; + if (built) { + RUN_ELEVATED({ + rc = signals::send_to_group_id(GID_TEST_GROUP, signals::SIGTERM); + }); + } + + EXPECT_TRUE(built); + EXPECT_EQ(rc, signals::OK); + + uint32_t reached = 0; + for (uint32_t i = 0; i < GID_GROUP_COUNT; i++) { + if (g_gid_groups[i] && (g_gid_groups[i]->sig.shared_pending.load_relaxed() + & signals::sig_bit(signals::SIGTERM))) { + reached++; + } + } + EXPECT_EQ(reached, GID_GROUP_COUNT); + + RUN_ELEVATED({ + for (uint32_t i = 0; i < GID_GROUP_COUNT; i++) { + if (g_gid_leaders[i] && g_gid_groups[i]) { + sched::g_task_registry.remove(*g_gid_leaders[i]); + } + if (g_gid_groups[i]) heap::kfree_delete(g_gid_groups[i]); + if (g_gid_leaders[i]) heap::kfree_delete(g_gid_leaders[i]); + g_gid_groups[i] = nullptr; + g_gid_leaders[i] = nullptr; + } + }); +} diff --git a/kernel/tests/sync/futex.test.cpp b/kernel/tests/sync/futex.test.cpp index cb46e007..e555b54f 100644 --- a/kernel/tests/sync/futex.test.cpp +++ b/kernel/tests/sync/futex.test.cpp @@ -195,10 +195,12 @@ TEST(futex, killed_thread_unblocks) { g_kill_entered.store_relaxed(0); g_kill_task = nullptr; + rc::strong_ref pin; RUN_ELEVATED({ g_kill_task = sched::create_kernel_task( kill_waiter_fn, nullptr, "ftx_kill"); ASSERT_NOT_NULL(g_kill_task); + pin = sched::task_ref(g_kill_task); sched::enqueue(g_kill_task); }); diff --git a/kernel/tests/sync/poll.test.cpp b/kernel/tests/sync/poll.test.cpp index 0f964773..90fee483 100644 --- a/kernel/tests/sync/poll.test.cpp +++ b/kernel/tests/sync/poll.test.cpp @@ -373,9 +373,11 @@ TEST(poll, kill_pending_wakes_poll) { g_kill_done.store_relaxed(0); sched::task* t = nullptr; + rc::strong_ref pin; RUN_ELEVATED({ t = sched::create_kernel_task(kill_poll_fn, nullptr, "poll_kill"); ASSERT_NOT_NULL(t); + pin = sched::task_ref(t); sched::enqueue(t); });