Skip to content

fix(agc): harden host import construction and mutation waits - #7339

Merged
oneandonlydean merged 1 commit into
boykopovar:mainfrom
oneandonlydean:rescue/2779
Oct 18, 2026
Merged

oneandonlydean merged 1 commit into
boykopovar:mainfrom
oneandonlydean:rescue/2779

Conversation

@oneandonlydean

Copy link
Copy Markdown
Collaborator

Repost of #2779 by @nikhilpanchal1, which the intake bot closed on 2026-10-10 under the open-PR limit ("intake paused"). main has moved a lot since (GuestBufferMemory.cpp has doubled), so this is a hand re-apply of the same change onto current main; the commit keeps the original author.

What

Follow-up to #2575 (MarcZX100's fix for #2352). Incomplete Vulkan host imports are cleaned up before their allocation pin is released: a guard owns the buffer, memory and Windows alias of an import under construction until the entry is published in the table, so a throw in between (a failed settle, a bad_alloc on insert) cannot leak them. The mutation waiter no longer swallows errors: WaitForLeasesAndImports propagates them, the registry lock is restored before the error leaves the pin loop, and a pinned mutation requested from inside an active GPU completion is rejected instead of waiting on itself. The waiter is installed centrally when an import is built, so descriptor-first imports are covered. New tests (HostImportLifetime.cpp, GuestMemoryLifetime.cpp, test_guest_memory_lifetime.py, plus fixtures) cover each of these.

Changed versus the original #2779

  • GuestBufferMemory.cpp: re-applied by hand on top of main's current code. Taken over: the ImportConstruction guard in importAllocation, WaitForLeasesAndImports without noexcept and without its two try/catch wrappers, the Recorder::InCompletion() rejection, and ensurePinWaiter() at the start of importAllocation. libc/GuestAllocations.cpp is unchanged from the original (restores the registry lock when the waiter throws).
  • Not taken over: Recorder::ReleaseCompletedKeeps() (main already releases through FlushDeferredReleases(), which WaitForLeasesAndImports now calls); the std::exchange change in bindImport, which main's code does not need; and the two edits in importChunk / createChunkedDmaBufImport, because publishing the buffer handle only after a successful create (as main does) is safer than writing it straight into the entry, and setting the chunk's free callback before vkAllocateMemory could free an undefined handle after a failed allocation.
  • tests/Recorder.cpp: only the HostImportLifetime.hpp include and the two new --host-import-*-only modes are added. The original also made unitShadowTests skip instead of silently pass, but main's version of that test and its retire assertions have since changed, so that part is left out.
  • tests/HostImportLifetime.cpp: one line added to deferredRetirement, fixture.recorder->FlushDeferredReleases() after the sync. main now keeps a retired import through the GPU hold until a flush, so the test has to flush before it expects the Vulkan free on the release thread.
  • CMakeLists.txt: the original's test registrations are kept; the two conflicts were adjacent additions and both sides are kept.
  • Open fix(agc): keep failed import handles out of cleanup #7082 (keep failed import handles out of cleanup) fixes the failed-handle part of bindImport and overlaps this only in that area; nothing here conflicts with it textually.

Tested

Linux x86-64, GCC, Release, NVIDIA GeForce RTX 4090 (real Vulkan), on this branch rebased onto main (51cee90): build OK; 24 of 24 tests passed (agc_driver_host_import_lifetime, agc_driver_host_import_mutation_race, agc_driver_guest_memory_lifetime in copy/direct/locked/heap/heap_uncached/converted modes, agc_unit_shadow, agc_driver_unit_shadow, agc_host_import_retire(ment) and the other host-import tests). Control: with GuestBufferMemory.cpp and GuestAllocations.cpp reverted to main, --host-import-lifetime-only fails ("still pinned ... waited 2000 ms") and --host-import-mutation-race-only fails ("a failed pin waiter did not restore the registry lock"). Not run: the full suite, Windows, DMA-buf chunk paths, Linux write-watch tests that need userfaultfd.

Checklist

  • Based on current main; no other open PR implements the same functions (the open fix(agc): keep failed import handles out of cleanup #7082 covers only the failed-handle cleanup in bindImport, which this does not touch)
  • One topic per PR; follow-ups go in a new PR
  • No comments in code except technical debt
  • Unimplemented paths throw (NotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file
  • No notes, investigation .md files or images added to the repository (attach them to this PR)
  • New third-party code is a submodule built from source
  • Generic behaviour, not specific to one title
  • Depends on: none
  • Fixes: none
  • Unblocks: none
  • GPUs tested: NVIDIA GeForce RTX 4090, driver 610.57.04, CachyOS Linux (the new host-import tests ran on it)
  • AI-assisted: yes

@oneandonlydean oneandonlydean self-assigned this Oct 18, 2026
@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Oct 18, 2026
Guard incomplete imports and preserve mutation waiter errors and progress.

Follow-up to boykopovar#2575; refs boykopovar#2352.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants