Skip to content

fix(runtime): split the runtime to avoid ref cycle#911

Open
Berrysoft wants to merge 2 commits into
compio-rs:masterfrom
Berrysoft:fix/runtime-ref-cycle
Open

fix(runtime): split the runtime to avoid ref cycle#911
Berrysoft wants to merge 2 commits into
compio-rs:masterfrom
Berrysoft:fix/runtime-ref-cycle

Conversation

@Berrysoft
Copy link
Copy Markdown
Member

@Berrysoft Berrysoft commented May 7, 2026

Closes #909

I think this solution is better. Let's see the ASan results.

cc: @paddor

@Berrysoft Berrysoft self-assigned this May 7, 2026
@Berrysoft Berrysoft requested a review from Copilot May 7, 2026 16:19
@github-actions github-actions Bot added bug Something isn't working package: runtime Related to compio-runtime labels May 7, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refactors compio-runtime to break a runtime/task reference cycle by splitting Runtime into separately reference-counted components and updating cancellation/submission/timer futures to hold driver/timer handles rather than a full Runtime clone. It also adds a regression test asserting that detached tasks are dropped when the runtime is dropped, even if they hold a CancelToken.

Changes:

  • Refactor Runtime to hold Rc<Executor> and Rc<RefCell<Proactor>> (and timer runtime) directly, with new current_driver() / current_timer_runtime() helpers.
  • Update CancelToken, Submit, SubmitMulti, and timer futures to use the shared driver/timer runtime handles and move driver polling helpers into future/mod.rs.
  • Add a new drop regression test to detect potential Rc cycles keeping tasks alive across runtime drop.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
compio-runtime/tests/drop.rs Adds a regression test ensuring a detached task holding a CancelToken is dropped when the runtime drops.
compio-runtime/src/time.rs Moves timer creation logic into time.rs and makes timer futures own an Rc<RefCell<TimerRuntime>>; adds TimerRuntime::poll_timer.
compio-runtime/src/lib.rs Splits Runtime into separate Rc fields, adjusts submit APIs to pass driver handles, and changes drop logic to key off executor refcount.
compio-runtime/src/future/stream.rs Changes SubmitMulti to store the driver handle and use shared poll/submit helpers.
compio-runtime/src/future/mod.rs Introduces shared internal helper functions for submit/poll operations against Proactor.
compio-runtime/src/future/future.rs Changes Submit to store the driver handle and use shared poll/submit helpers.
compio-runtime/src/cancel.rs Changes CancelToken internals to store the driver handle (instead of Runtime) and adjusts debug output accordingly.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread compio-runtime/src/time.rs
Comment thread compio-runtime/src/lib.rs
@Berrysoft Berrysoft requested review from George-Miao and fantix May 7, 2026 16:38
@Berrysoft Berrysoft added this to the v0.19 milestone May 13, 2026
@Berrysoft Berrysoft force-pushed the fix/runtime-ref-cycle branch from 453ed63 to db14930 Compare May 14, 2026 13:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working package: runtime Related to compio-runtime

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants