Skip to content

Document thread-safety caveat for get_outstanding_allocations()#2418

Draft
rpathade wants to merge 1 commit into
rapidsai:mainfrom
rpathade:doc-thread-safety-get-outstanding-allocations
Draft

Document thread-safety caveat for get_outstanding_allocations()#2418
rpathade wants to merge 1 commit into
rapidsai:mainfrom
rpathade:doc-thread-safety-get-outstanding-allocations

Conversation

@rpathade
Copy link
Copy Markdown

Description

Closes #2395

Add @warning Doxygen comments to get_outstanding_allocations() in both the public and impl headers, noting that the returned reference is a view of internal mutable state and is only safe when the caller ensures no concurrent allocations or deallocations.

This implements Option 1 from the issue (Document Caller Synchronization).

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

@copy-pr-bot
Copy link
Copy Markdown

copy-pr-bot Bot commented May 25, 2026

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 25, 2026

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 380c544a-4805-4774-bca4-f84841ab2590

📥 Commits

Reviewing files that changed from the base of the PR and between 5fd6cee and 1e310d8.

📒 Files selected for processing (2)
  • cpp/include/rmm/mr/detail/tracking_resource_adaptor_impl.hpp
  • cpp/include/rmm/mr/tracking_resource_adaptor.hpp

📝 Walkthrough

Summary by CodeRabbit

Release Notes

  • Documentation
    • Improved documentation clarifying thread-safety considerations and safe usage patterns for memory allocation tracking functionality, including warnings about concurrent operations.

Walkthrough

Updated Doxygen documentation for tracking_resource_adaptor::get_outstanding_allocations() in both the public header and implementation detail header to add thread-safety warnings. The warnings clarify that the returned map is a view of mutable internal state and is only safe under no-concurrent-allocation/deallocation conditions.

Changes

Outstanding Allocations Thread-safety Documentation

Layer / File(s) Summary
Thread-safety documentation for get_outstanding_allocations()
cpp/include/rmm/mr/tracking_resource_adaptor.hpp, cpp/include/rmm/mr/detail/tracking_resource_adaptor_impl.hpp
Added Doxygen @warning blocks documenting that get_outstanding_allocations() returns a view of internal mutable state from the tracking allocations map, safe only when the caller ensures no concurrent allocations or deallocations occur through the adaptor.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Possibly related PRs

  • rapidsai/rmm#2304: Addresses related concurrency fixes in tracking_resource_adaptor's allocation/deallocation behavior that complement this documentation update.

Suggested labels

non-breaking

Suggested reviewers

  • bdice
  • wence-
  • ttnghia
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: adding documentation for the thread-safety caveat of get_outstanding_allocations().
Description check ✅ Passed The description clearly explains the changes: adding @warning Doxygen comments to document thread-safety requirements for get_outstanding_allocations().
Linked Issues check ✅ Passed The PR implements Option 1 from issue #2395 by adding @warning Doxygen comments documenting that get_outstanding_allocations() returns internal mutable state only safe with external synchronization.
Out of Scope Changes check ✅ Passed All changes are in-scope documentation additions to tracking_resource_adaptor headers addressing thread-safety warnings as specified in the linked issue.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

Make tracking resource outstanding allocations access thread-safe

1 participant