Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes a use-after-free in compio-executor task wake/scheduling teardown, and adjusts remote JoinHandle polling/waker-setting interactions to avoid races while also reducing unnecessary wake/spin behavior.
Changes:
- Reworks task teardown to null out
Sharedearlier and waits for in-flight remote scheduling before droppingSharedto prevent UAF. - Tweaks remote polling waker-setting critical section to correctly handle “completed while setting waker” interleavings.
- Adds an
enable_logfeature for opt-in test logging and updates the dev shell inputs.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| flake.nix | Adds cargo-flamegraph (and reorders python315) in the dev shell inputs. |
| compio-executor/tests/executor.rs | Gates test log initialization behind feature = "enable_log" and inlines EnvFilter path. |
| compio-executor/src/task/remote.rs | Refines remote waker-setting logic to handle completion/cancellation during the critical section. |
| compio-executor/src/task/mod.rs | Removes busy-spin on SETTING_WAKER, changes wake conditions, nulls shared in Task::drop, and renames/repurposes teardown waiting API. |
| compio-executor/src/queue.rs | Updates executor teardown to call drop() then wait_for_scheduling() per drained task. |
| compio-executor/Cargo.toml | Introduces enable_log feature mapping to compio-log/enable_log; removes the prior dev-dep feature usage. |
Comments suppressed due to low confidence (1)
compio-executor/src/task/mod.rs:321
wait_for_schedulingassumes that new schedulers will “see the null pointer and return early”, but this function no longer nullsheader.shareditself. To make this invariant harder to violate, consider either (a) moving theshared.store(null, Release)back intowait_for_scheduling, or (b) documenting/enforcing the precondition (e.g., adebug_assert!(header.shared.load(Relaxed).is_null())) so future call sites don’t accidentally reintroduce UAF risk.
/// Wait for wakers to finish scheduling, if any. This is necessary for
/// `Executor` to drop `Shared` since scheduling requires it.
pub(crate) fn wait_for_scheduling(&self) {
let header = self.header();
// Wait for any ongoing scheduling to complete.
// We MUST do this to prevent use-after-free when we drop Shared.
// This is safe in Executor::drop context because:
// 1. All tasks have been cleared, so no new scheduling from task execution
// 2. Only external wakers (from other threads) might still be scheduling
// 3. Those wakers will see the null pointer and return early
// 4. We only need to wait for ones that already loaded the pointer
while header.state.load::<Strong>().is_scheduling() {
crate::hint::spin_loop();
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
b34c77c to
5416095
Compare
5fb1b78 to
85cbe06
Compare
85cbe06 to
f0854ca
Compare
Berrysoft
approved these changes
Apr 10, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR solves an UAF bug found by @fantix, and optimizes waking-up logic when a task has finished running and the JoinHandle is remotely polling at the same time.