Skip to content

Avoid tracking counter underflow#2404

Merged
rapids-bot[bot] merged 2 commits into
rapidsai:mainfrom
bdice:pr-avoid-tracking-counter-underflow
May 22, 2026
Merged

Avoid tracking counter underflow#2404
rapids-bot[bot] merged 2 commits into
rapidsai:mainfrom
bdice:pr-avoid-tracking-counter-underflow

Conversation

@bdice
Copy link
Copy Markdown
Collaborator

@bdice bdice commented May 17, 2026

Description

Prevents tracking_resource_adaptor from decrementing allocated_bytes_ for untracked deallocations. The counter is now reduced only when a pointer is found and erased from the tracked allocation map.

I verified that TrackingTest.UntrackedDeallocationDoesNotUnderflowAllocatedBytes fails without the fix.

Checklist

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

@bdice bdice added bug Something isn't working non-breaking Non-breaking change labels May 17, 2026
@copy-pr-bot
Copy link
Copy Markdown

copy-pr-bot Bot commented May 17, 2026

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
Copy link
Copy Markdown

coderabbitai Bot commented May 17, 2026

Review Change Stack

📝 Walkthrough

Walkthrough

The PR fixes an underflow bug in tracking_resource_adaptor by moving the allocated_bytes_ decrement operation inside the nonzero-bytes guard in the deallocate method. A new test validates that untracked deallocations through nested adaptors do not underflow accounting.

Changes

Untracked deallocation fix

Layer / File(s) Summary
Untracked deallocation accounting fix
cpp/src/mr/detail/tracking_resource_adaptor_impl.cpp, cpp/tests/mr/tracking_mr_tests.cpp
deallocate moves allocated_bytes_ -= bytes inside the if (bytes != 0) block after removing the allocation record, preventing underflow when deallocations occur through a different tracking adaptor. Test validates both adaptors report zero allocations after outer allocation and inner deallocation of the same pointer.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • rapidsai/rmm#2316: Adjusts tracking_resource_adaptor deallocation accounting to prevent incorrect allocated_bytes_ updates in zero/untracked deallocation edge cases.
  • rapidsai/rmm#2304: Modifies tracking_resource_adaptor_impl deallocate logic and extends tracking_mr_tests.cpp to cover deallocation correctness scenarios.

Suggested reviewers

  • ttnghia
  • PointKernel
  • harrism
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: preventing counter underflow in the tracking resource adaptor, which is exactly what the changeset addresses.
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 The pull request description clearly explains the fix: preventing allocated_bytes_ from being decremented for untracked deallocations, with specific details about when the counter is reduced.

✏️ 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.

@bdice bdice moved this to In Progress in RMM Project Board May 21, 2026
@bdice bdice marked this pull request as ready for review May 21, 2026 14:21
@bdice bdice requested a review from a team as a code owner May 21, 2026 14:21
@bdice bdice requested review from lamarrr and rongou May 21, 2026 14:21
@bdice bdice self-assigned this May 21, 2026
@bdice
Copy link
Copy Markdown
Collaborator Author

bdice commented May 22, 2026

/merge

@rapids-bot rapids-bot Bot merged commit 75c4f1b into rapidsai:main May 22, 2026
85 checks passed
@github-project-automation github-project-automation Bot moved this from In Progress to Done in RMM Project Board May 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working non-breaking Non-breaking change

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants