Skip to content

fix(agc): release host imports before the device is destroyed - #1239

Merged
adriadam10 merged 2 commits into
boykopovar:mainfrom
awake-devel:fix/agc-release-host-imports
Oct 8, 2026
Merged

adriadam10 merged 2 commits into
boykopovar:mainfrom
awake-devel:fix/agc-release-host-imports

Conversation

@awake-devel

@awake-devel awake-devel commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

What

Closes #1236.

VulkanDevice::State::~State releases the device's caches before vkDestroyDevice but never emptied the host import registry (Imports() in GuestBufferMemory.cpp, a function-local static). Every import still registered at teardown kept its VkBuffer and VkDeviceMemory past vkDestroyDevice, which the validation layer reports as VUID-vkDestroyDevice-device-05137. The next device then freed them through the destroyed device's handle in refreshImports, and that crashes: a second device importing the same block after the first was destroyed ends the process with SIGILL in the NVIDIA driver (SIGABRT under the validation layer). In the app the device is only destroyed in Driver::Shutdown, and the replacement at the first flip keeps the old device alive (#59), so no title reaches the crash today; tests and any later device recreation do.

  • ClearHostImports(VkDevice), beside ClearImageMirrors: under the registry lock, when the registry belongs to the device, destroys each import's buffer and memory (and unmaps its alias on Windows) as the device-change path of refreshImports does, then clears imports and failed, resets device and refreshedGeneration and bumps epoch.
  • ~State calls it right after ClearImageMirrors, once the recorder (with its retired imports), the resource cache and copiedWriters are gone.
  • New agc_driver_host_import_teardown: a device fills part of a registered guest block through VulkanDevice::FillBuffer, which imports it; the test checks that the fill reached guest memory and that the import exists, destroys the device and checks that the registry no longer holds an import for it. A second device then does the same with the same block, the sequence that crashed. It is skipped on devices without host imports.

tests/Recorder.cpp builds its own VkDevice and would need the same call before its vkDestroyDevice; it is not registered in ctest, and #1046 and #1210 rework that harness, so it is left alone here.

Tested

Linux (Arch, GCC 16.2.1), GeForce GTX 1050 Ti Max-Q, NVIDIA 580.178.04.

  • Full ctest: 400/400 pass (the 399 tests of main and the new one).
  • Full ctest under VK_LAYER_KHRONOS_validation 1.4.363.0 (core validation): 400/400 pass with no validation message. On main the 23 execution tests listed in AGC: host imports are not released before vkDestroyDevice (VUID-vkDestroyDevice-device-05137) #1236 report VUID-vkDestroyDevice-device-05137.
  • With the ClearHostImports call removed, the new test fails with the guest block's import outlived the first device and the layer reports 2 leaked objects; with it, the test passes.
  • The crash, measured with a variant of the test that goes on past that check: without this change the second device's import ends the process with SIGILL (exit 132), or SIGABRT (exit 134) under the layer; with it, both devices import, fill and release the block with no validation message.
  • Not tested on AMD or Windows; the _WIN32 alias unmapping follows refreshImports and was not built here.

Checklist

  • Based on current main; no other open PR implements the same functions
  • 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 in TechnicalDebt
  • 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
  • AI-assisted: yes

🤖 Generated with Claude Code

https://claude.ai/code/session_018RNrNcixatky8U4LfC7Ew5

@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Oct 7, 2026
awake-devel and others added 2 commits October 7, 2026 23:41
The host import registry is a function-local static that device
teardown never emptied, so every import still registered kept its
VkBuffer and VkDeviceMemory past vkDestroyDevice
(VUID-vkDestroyDevice-device-05137 in 23 execution tests).
ClearHostImports destroys the device's imports and resets the
registry; ~State calls it next to ClearImageMirrors.

Closes boykopovar#1236

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018RNrNcixatky8U4LfC7Ew5
Without the host import release, a device created after another was
destroyed freed the leftover imports through the destroyed device and
the process crashed (SIGILL on NVIDIA). The teardown test now runs a
second device over the same guest block once the first is gone.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018RNrNcixatky8U4LfC7Ew5
@adriadam10

Copy link
Copy Markdown
Collaborator

Checked on Linux (main + PR): full ctest 431/431; agc_driver_host_import_teardown passes, and fails with the ClearHostImports call reverted. ClearHostImports mirrors the device-change path of refreshImports (destroy buffer/memory, unmap alias on Windows, clear imports/failed, bump epoch). Linux, Windows and conventions CI are green on this head; merging.

@adriadam10
adriadam10 merged commit 96614df into boykopovar:main Oct 8, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agc libSceAgc*

Projects

None yet

Development

Successfully merging this pull request may close these issues.

AGC: host imports are not released before vkDestroyDevice (VUID-vkDestroyDevice-device-05137)

2 participants