Skip to content

fix(vram): roll back partial allocation batches - #10

Open
Neonstudio28 wants to merge 1 commit into
knewnothing-git:mainfrom
Neonstudio28:fix/allocation-batch-rollback
Open

fix(vram): roll back partial allocation batches#10
Neonstudio28 wants to merge 1 commit into
knewnothing-git:mainfrom
Neonstudio28:fix/allocation-batch-rollback

Conversation

@Neonstudio28

Copy link
Copy Markdown

Summary

GpusionCreateAllocation currently leaks allocations created earlier in a batch when a later tracking or backing-memory allocation fails. It also leaves VRAM accounting inflated even though the call returns STATUS_NO_MEMORY.

This change:

  • routes both allocation-failure paths through a rollback block;
  • frees only descriptors completed by the failed call through the existing destroy path;
  • clears rolled-back output handles and sizes;
  • preserves allocations that existed before the failed batch.

Closes #9

Validation

tests/allocation-rollback.test.c injects failure at all six tracking and backing allocation points in a three-item batch. It also verifies:

  • a pre-existing allocation remains intact;
  • VRAM and allocation counters are balanced after rollback;
  • the same descriptors can be retried successfully;
  • final cleanup releases every allocation.

Executed locally on macOS arm64:

cc -std=c99 -Wall -Wextra -Werror -fsanitize=address,undefined tests/allocation-rollback.test.c -o /tmp/gpusion-allocation-test
/tmp/gpusion-allocation-test
PASS: six allocation failures, retries, existing allocation preservation

git diff --check passes. Windows/WDK and kernel execution were not tested. The test suppresses the independent diagnostic format-string defect covered by PR #8.

Prepared with AI assistance; the reproduction and fix were reviewed and executed locally.

Please confirm whether this fix is eligible for the documented bounty. If accepted, payout details can be supplied privately by UPI or bank transfer.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The rollback logic correctly destroys only prior successful allocations in the failing batch and the new test exercises all failure points plus retry and accounting invariants.

Pull request overview

Fixes GpusionCreateAllocation batch semantics so that if any allocation in the batch fails, the function rolls back only the allocations successfully created earlier in the same call (including VRAM/accounting), while preserving allocations created by prior calls.

Changes:

  • Route tracking-struct and backing-memory allocation failures through a unified rollback path in GpusionCreateAllocation.
  • Roll back previously-created allocations via GpusionDestroyAllocation and clear the corresponding output driver-data fields.
  • Add a portable regression test that injects failures at each allocation point and validates rollback + retry behavior.
File summaries
File Description
tests/allocation-rollback.test.c Adds a standalone harness that injects allocation failures and asserts rollback, accounting balance, and successful retry.
driver/vram/vram_proxy.c Implements rollback-on-failure for partial batches by destroying only allocations completed earlier in the same call and clearing output handles/sizes.
Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 0
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@Neonstudio28

Neonstudio28 commented Sep 9, 2026

Copy link
Copy Markdown
Author

Follow-up on PR #10: the six failure-injection cases pass under AddressSanitizer/UndefinedBehaviorSanitizer, and the patch preserves a pre-existing allocation while rolling back only the completed descriptors from the failed batch. GitHub currently marks the forked PR CI run as action_required with no jobs started, so maintainer approval may be needed to run the repository workflow. Please confirm whether the fix is eligible for the documented bug-fix bounty and review when convenient. I can adapt the regression test to the project harness if needed; payout details can be sent privately after merge.

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.

fix(vram): roll back completed allocations when a batch fails

2 participants