Skip to content

refactor!: kill direct tokio sync coupling in hot paths - #2394

Merged
gold-silver-copper merged 3 commits into
mainfrom
bevy-prep/kill-tokio-sync
Aug 21, 2026
Merged

refactor!: kill direct tokio sync coupling in hot paths#2394
gold-silver-copper merged 3 commits into
mainfrom
bevy-prep/kill-tokio-sync

Conversation

@gold-silver-copper

@gold-silver-copper gold-silver-copper commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Bevy-prep PR 1: rig's hot paths no longer touch tokio primitives, so a tokio runtime is not required for streaming pause/resume, tool-server registration, or the copilot/chatgpt auth caches. Bevy ships its own task pools (bevy_tasks) and Bevy users won't have a tokio runtime; tokio's sync primitives were runtime-agnostic in practice, but they kept a direct tokio coupling in paths that don't need it. Adds zero new direct dependencies: every replacement primitive comes from futures, which was already a dependency.

Changes

Streaming pause/resume (rig-core/src/streaming/mod.rs). PauseControl drops its tokio::sync::watch channel for a single shared Arc<PauseState { paused: AtomicBool, waker: futures::task::AtomicWaker }>, and is now Clone. The stream is the only consumer (poll_next takes Pin<&mut Self>), which is exactly the single-waiter shape AtomicWaker is built for — a watch channel is multi-waiter machinery this call site never needed. poll_next integrates directly via the register-then-recheck AtomicWaker protocol, which deletes the boxed resume_wait future and its ResumeWait type aliases entirely. Both #2258 invariants are preserved and remain pinned by the existing tests: no lost wakeup (resume clears the flag before waking; poll registers its waker before re-reading the flag) and no busy re-wake while paused (H7).

Tool server (rig-agent/src/tool/server.rs). ToolServerHandle's tokio::sync::RwLock becomes std::sync::RwLock: all eight lock sites are clone-under-lock or sync mutations inside sync closures — no guard ever crossed an await. Poisoning is recovered via PoisonError::into_inner in one pair of private accessors (the short sync critical sections can't leave the registry logically torn). The registration-only methods de-async — add_tool, add_dynamic_tool, add_portable_dynamic_tool, append_toolset, remove_tool — breaking change, MIGRATING.md entry included; execution/snapshot paths stay async. The deadlock-regression test that wrapped add_tool in tokio::time::timeout now calls it directly: with a sync method, a lock held across tool execution would hang the test harness, which is the same pin.

Auth caches (providers/copilot/auth, providers/chatgpt/auth). The Arc<tokio::sync::Mutex<()>> unit locks (held across awaits — network calls and device-flow polling — so a std mutex is not an option) become Arc<futures::lock::Mutex<platform::PlatformAuthenticator>>: the lock now wraps the token/key cache state it actually serializes instead of guarding code, and futures::lock works on both the native and wasm halves. (futures::lock::Mutex over async_lock::Mutex: equivalent for this slow, low-contention refresh path, and it avoids a new direct dependency.)

Device-flow sleeps. tokio::time::sleep in the copilot/chatgpt device flows only compiled via feature unification — rig-core builds tokio without the time feature. Both sites now use a new wasm_compat::sleep helper built on futures_timer::Delay, next to the existing wasm_compat::timeout.

Deliberately out of scope

  • rig-core's tokio dependency line stays: reqwest pulls tokio transitively regardless, and the feature-gated openai realtime websocket still needs it. Removing it is the tail end of a planned transport-crate split.
  • tool/rmcp.rs and integrations/discord_bot.rs keep their tokio locks — the rmcp feature pulls tokio via the rmcp SDK anyway (rmcp.rs has a real tokio::spawn).

Verification

  • cargo check --workspace --all-features --all-targets and cargo clippy (same flags): clean.
  • cargo test -p rig-core --all-features (1799 passed) and -p rig-agent --all-features (598 passed), zero failures; the Canonical stream grammar: mandatory identity, one accumulator, decode-then-validate, and a wire-conformance corpus #2258 pause/resume tests and the tool-server concurrency tests pass unchanged.
  • CI's wasm command cargo check -p rig-core --all-features --target wasm32-unknown-unknown: clean.
  • Residual non-test tokio::sync/tokio::time grep over rig-core/rig-agent: only test code, rmcp-gated code, and the websocket module.

Bevy-prep PR 1: rig's hot paths no longer touch tokio primitives, so a
tokio runtime is not required for streaming pause/resume, tool-server
registration, or the copilot/chatgpt auth caches.

- streaming: PauseControl drops its tokio::sync::watch channel for one
  Arc<PauseState { AtomicBool, futures::task::AtomicWaker }>. poll_next
  integrates directly via the register-then-recheck AtomicWaker protocol,
  deleting the boxed resume_wait future; both #2258 H7 invariants (no
  busy re-wake, no lost resume race) are preserved and pinned by the
  existing tests. PauseControl is now Clone.

- tool server: ToolServerHandle's tokio RwLock becomes std::sync::RwLock
  (no guard ever crossed an await; all eight sites are clone-under-lock
  or sync mutations). Poisoning is recovered via PoisonError::into_inner
  in one pair of private accessors. Registration-only methods de-async:
  add_tool, add_dynamic_tool, add_portable_dynamic_tool, append_toolset,
  remove_tool (MIGRATING.md entry included). rmcp.rs and discord_bot.rs
  keep tokio locks: the rmcp feature pulls tokio via the SDK regardless.

- auth caches: the copilot/chatgpt Mutex<()> becomes
  async_lock::Mutex<PlatformAuthenticator> — the lock now wraps the
  state it serializes (token/key caches) instead of guarding code, and
  async-lock works on both native and wasm halves.

- device-flow sleeps: tokio::time::sleep only compiled via feature
  unification (rig-core builds tokio without "time"); both call sites
  now use a new wasm_compat::sleep built on futures_timer, next to the
  existing timeout helper.

Residual non-test tokio in rig-core is the openai realtime websocket
(feature-gated, moves out in the transport-crate split) and test-only
code. Verification: cargo check/clippy --workspace --all-features
--all-targets clean; rig-core (1799) and rig-agent (598) tests pass;
wasm32-unknown-unknown check (CI's command) passes.
…nc-lock

The futures crate is already a dependency and its async mutex is a
drop-in for this slow, low-contention path (token refresh), so the PR
now adds zero new direct dependencies.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant