Allow cuvsRMMHostFree on a different thread than cuvsRMMHostAlloc

[bdice]

cuvsRMMHostAlloc and cuvsRMMHostFree used a thread_local std::unique_ptr<rmm::mr::pinned_host_memory_resource> that was created only on a thread's first cuvsRMMHostAlloc call. Freeing a buffer on any other thread, for example after a binding moves the owning object to another thread, dereferenced a null pointer. pinned_host_memory_resource is stateless, so this uses one process-wide instance instead.

The new cuvs_c_host_alloc_test frees a buffer on a different thread than the one that allocated it, and runs concurrent allocations and frees across 8 threads.

This was found while investigating the Rust parallel cargo test SIGSEGV in #2233, but it has not been shown to cause that report. The crash did not reproduce in about 6,700 parallel iterations (#2818). Part of #2783.

The same investigation found a data race on the function-local static prime counter in balanced k-means adjust_centers(). That counter is removed by #2804, so it is not changed here.

[bdice]

/ok to test b117ab8

[copy-pr-bot]

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository: NVIDIA/cuvs/.coderabbit.yaml
• Review profile: CHILL
• Plan: Enterprise
• Run ID: 3983f779-bcd9-45a1-803f-6e93fbf8fbd3

---

📥 Commits

Reviewing files that changed from the base of the PR and between 24b0b17 and b117ab8.

---

📒 Files selected for processing (3)

• c/src/core/c_api.cpp
• c/tests/CMakeLists.txt
• c/tests/core/host_alloc.cpp

---

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.

---

---

📝 Summary

Summary by CodeRabbit

• Bug Fixes
  • Pinned host memory can now be freed from a different thread than the one that allocated it, including when multiple threads allocate and free buffers concurrently.
• Tests
  • Added coverage for cross-thread and concurrent allocation and deallocation.

Walkthrough

The host allocation and free APIs now use one process-wide pinned host memory resource. New tests check freeing buffers from another thread, including 400 allocations across eight threads.

Changes

Cross-thread host allocation

Layer / File(s)	Summary
Use a shared pinned host resource c/src/core/c_api.cpp	The host allocation and free APIs now use a process-wide resource accessor instead of a thread-local resource.
Test cross-thread frees c/tests/core/host_alloc.cpp, c/tests/CMakeLists.txt	Tests cover a single cross-thread free and 400 allocations across eight threads, with each thread freeing buffers allocated by another. CMake adds the test target.

Priority: ⬇️ Low

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

Change: Bug fix

Suggested reviewers: achirkin

---

Merge Risk: ⚪ Minimal · up to b117a

No concrete merge-blocking issue was identified. The pinned resource’s concurrency guarantee remains unverified.

Pre-merge checks · 4 · 1

Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 2 files. (1 skipped: 1…	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks (4 passed)

Check name	Status	Explanation
Title check		The title clearly and concisely describes the primary change: allowing cuvsRMMHostFree to run on a different thread from cuvsRMMHostAlloc.
Description check		The description directly explains the thread-local resource issue, the process-wide resource fix, and the added tests.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

---

Full details: Docstring Coverage

Explanation

Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 2 files. (1 skipped: 1 unsupported.)

---

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

---

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

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