fix(lance-core): avoid aliasing &mut Runtime in global_cpu_runtime by LuciferYang · Pull Request #7682 · lance-format/lance

LuciferYang · 2026-07-08T11:53:38Z

What & why

lance-core keeps a process-global CPU thread-pool runtime behind CPU_RUNTIME: AtomicPtr<Runtime>. global_cpu_runtime() dereferenced that raw pointer into &'static mut Runtime. Its only caller, spawn_cpu, runs concurrently on many worker threads, so multiple &'static mut references to the same Runtime could be live at once — a violation of &mut uniqueness, which is undefined behavior on the Rust abstract machine even though every Runtime method actually used takes &self and never mutates. It is latent UB the compiler is permitted to miscompile under noalias assumptions (Miri flags it), not an observed crash today.

How & why it is correct

Return &'static Runtime and dereference with &*ptr instead. Three facts make this sound:

The only method called is Runtime::spawn_blocking, which takes &self — the &mut was never needed.
Runtime: Sync, so many threads may hold &'static Runtime concurrently without UB, which is exactly the aliasing the old code got wrong.
The runtime is created via Box::into_raw and never reclaimed, so the &'static lifetime is genuine (no use-after-free); atfork_tokio_child only nulls the pointer, it does not free.

Both unsafe derefs now carry // SAFETY: comments documenting these invariants.

Compatibility

Fully backward compatible, zero behavior change. global_cpu_runtime is private, so the return-type change is internal-only; spawn_cpu's public signature is unchanged. &mut Runtime already auto-reborrowed to &self for spawn_blocking, so narrowing to &Runtime is byte-for-byte identical at runtime — purely tightening a gratuitous, unsound &mut into a shared &.

Test plan

cargo test -p lance-core
cargo clippy -p lance-core --tests -- -D warnings
cargo fmt --all -- --check

codecov · 2026-07-08T12:37:12Z

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

westonpace

Just some minor nits, and let's get rid of the test. The comment even explains that the old code would still have passed. We have basic happy path testing of spawn_cpu implicitly in plenty of other tests.

westonpace · 2026-07-08T15:01:17Z

+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    // Drives many `spawn_cpu` calls across real worker threads. Each task calls
+    // `global_cpu_runtime()` on its own thread, so before the fix (`&'static mut
+    // Runtime`) their transient `&mut` references to the one shared runtime could be
+    // live simultaneously — aliasing `&mut`, which violates `&mut` uniqueness. The
+    // fix returns a shared `&'static Runtime`, which many threads may hold at once.
+    //
+    // This is a functional/concurrency check, not a deterministic regression test:
+    // the aliasing was latent UB, so a plain `cargo test` passes on the old code too
+    // (the miscompilation it permits is not forced), and tokio's multi-thread runtime
+    // is not runnable under Miri. It guards against a future refactor breaking the
+    // concurrent path and documents the intended shared-reference contract.
+    #[tokio::test(flavor = "multi_thread", worker_threads = 4)]
+    async fn spawn_cpu_shared_runtime_is_concurrent_safe() {
+        let handles: Vec<_> = (0..64)
+            .map(|i| {
+                tokio::spawn(async move {
+                    spawn_cpu(move || Ok::<_, std::convert::Infallible>(i * 2))
+                        .await
+                        .unwrap()
+                })
+            })
+            .collect();
+        let mut results = Vec::with_capacity(handles.len());
+        for handle in handles {
+            results.push(handle.await.unwrap());
+        }
+        results.sort_unstable();
+        assert_eq!(results, (0..64).map(|i| i * 2).collect::<Vec<_>>());
+    }
+}


I don't think we need this test (I don't think it is actually testing anything is it?)

westonpace · 2026-07-08T15:04:19Z

+            // `Runtime` lives for the rest of the process — a `&'static` shared
+            // reference is valid. Multiple threads may hold this reference at once;
+            // that is sound because `Runtime` is `Sync` and every method used
+            // (`spawn_blocking`) takes `&self`.


Suggested change

// `Runtime` lives for the rest of the process — a `&'static` shared

// reference is valid. Multiple threads may hold this reference at once;

// that is sound because `Runtime` is `Sync` and every method used

// (`spawn_blocking`) takes `&self`.

// `Runtime` lives for the rest of the process

The first part of this comment is interesting (may not always be obvious to readers that static state is leaked on fork) but the second part seems to just be redefining the concept of a shared reference.

westonpace · 2026-07-08T15:04:35Z

+    // SAFETY: `new_ptr` was just obtained from `Box::into_raw`, so it is non-null,
+    // aligned, and points to a live `Runtime` that is never reclaimed. See the
+    // load branch above for why a shared `&'static` reference is the correct form.


Suggested change

// SAFETY: `new_ptr` was just obtained from `Box::into_raw`, so it is non-null,

// aligned, and points to a live `Runtime` that is never reclaimed. See the

// load branch above for why a shared `&'static` reference is the correct form.

// SAFETY: `new_ptr` was just obtained from `Box::into_raw`, so it is non-null,

// aligned, and points to a live `Runtime` that is never reclaimed.

global_cpu_runtime() returned &'static mut Runtime by dereferencing a process-global AtomicPtr<Runtime>. Its only caller, spawn_cpu, runs on many worker threads concurrently, so multiple &'static mut references to the same Runtime could be live at once — a violation of &mut uniqueness, which is undefined behavior on the abstract machine even though every Runtime method used takes &self and does not mutate. Return &'static Runtime and dereference with &*ptr instead. spawn_blocking takes &self, and Runtime is Sync, so many threads may share the reference soundly. The runtime is created via Box::into_raw and never reclaimed, so the &'static lifetime is genuine. Both unsafe derefs now carry SAFETY comments documenting these invariants.

coderabbitai · 2026-07-09T03:01:30Z

📝 Walkthrough

Walkthrough

The function global_cpu_runtime() in rust/lance-core/src/utils/tokio.rs is changed to return a shared &'static Runtime instead of &'static mut Runtime. Both unsafe pointer dereferences within the function are updated accordingly, with added safety comments explaining pointer provenance and lifetime guarantees.

Changes

Runtime accessor mutability change

Layer / File(s)	Summary
Shared reference conversion in global_cpu_runtime `rust/lance-core/src/utils/tokio.rs`	Return type changes from `&'static mut Runtime` to `&'static Runtime`; both unsafe dereferences change from `&mut ptr`/`&mut new_ptr` to `&ptr`/`&new_ptr`, with added SAFETY comments on pointer provenance and lifetime.

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the core fix in `global_cpu_runtime` by removing the aliased mutable `Runtime` reference.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

_{Comment @coderabbitai help to get the list of available commands.}

LuciferYang · 2026-07-09T03:19:17Z

Thanks for the review! @westonpace Addressed all three: dropped the test, and trimmed both SAFETY comments as suggested. Also rebased onto latest main.

westonpace

Thanks!

LuciferYang · 2026-07-10T14:15:07Z

Thank you @westonpace

github-actions Bot added the bug Something isn't working label Jul 8, 2026

wjones127 self-requested a review July 8, 2026 14:49

westonpace requested changes Jul 8, 2026

View reviewed changes

wjones127 removed their request for review July 8, 2026 16:02

LuciferYang force-pushed the fix/lance-core-cpu-runtime-aliasing-mut branch from b4179b3 to 4d2eef2 Compare July 9, 2026 03:01

LuciferYang requested a review from westonpace July 10, 2026 09:18

westonpace approved these changes Jul 10, 2026

View reviewed changes

westonpace merged commit 38f7bea into lance-format:main Jul 10, 2026
38 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(lance-core): avoid aliasing &mut Runtime in global_cpu_runtime#7682

fix(lance-core): avoid aliasing &mut Runtime in global_cpu_runtime#7682
westonpace merged 1 commit into
lance-format:mainfrom
LuciferYang:fix/lance-core-cpu-runtime-aliasing-mut

LuciferYang commented Jul 8, 2026 •

edited

Loading

Uh oh!

codecov Bot commented Jul 8, 2026

Uh oh!

westonpace left a comment

Uh oh!

westonpace Jul 8, 2026

Uh oh!

westonpace Jul 8, 2026

Uh oh!

westonpace Jul 8, 2026

Uh oh!

coderabbitai Bot commented Jul 9, 2026 •

edited

Loading

Walkthrough

Changes

Uh oh!

LuciferYang commented Jul 9, 2026

Uh oh!

westonpace left a comment

Uh oh!

Uh oh!

LuciferYang commented Jul 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

LuciferYang commented Jul 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What & why

How & why it is correct

Compatibility

Test plan

Uh oh!

codecov Bot commented Jul 8, 2026

Codecov Report

Uh oh!

westonpace left a comment

Choose a reason for hiding this comment

Uh oh!

westonpace Jul 8, 2026

Choose a reason for hiding this comment

Uh oh!

westonpace Jul 8, 2026

Choose a reason for hiding this comment

Uh oh!

westonpace Jul 8, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot commented Jul 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Uh oh!

LuciferYang commented Jul 9, 2026

Uh oh!

westonpace left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

LuciferYang commented Jul 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

LuciferYang commented Jul 8, 2026 •

edited

Loading

coderabbitai Bot commented Jul 9, 2026 •

edited

Loading