Synchronize and bound the Vulkan swapchain tracking (g_swaps)

g_swaps (vk_hook.cpp and the Vulkan layer) is pushed from the create-swapchain
detour and iterated by the present detour, which can run on different game
threads (Vulkan external sync is per-object, not global), and cleared on removal
from another thread -- all with no mutex. A push_back realloc could dangle the
SwapInfo* a concurrent find_swap/present is using. It was also never pruned, so a
game that recreates its swapchain each resize grew it without bound and could
match a recycled handle's stale images.

Add g_swaps_mutex around every access; the present detour now copies the matched
swapchain's fields out under the lock and captures without holding it (no GPU
submit under the lock, no dangling pointer). Create de-dups by handle and an LRU
cap (8) bounds growth -- the active swapchain is the newest, so it's never
evicted. Deliberately NOT hooking vkDestroySwapchainKHR: forwarding a destroy
incorrectly could break the game, and the de-dup + cap already bound growth and
defeat handle recycling.

Verified real by inspection (a concurrent-create+present Vulkan race isn't
deterministically reproducible in a test); validated by the full mock_game_test
Vulkan paths (capture + layer + too-late) staying green.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

README.md

@@ -114,9 +114,6 @@ From an in-depth review pass. Each item is fixed test-first (a failing test, the
 as its own commit; "verify" items are confirmed real before any change, and dropped if not.
 
 Correctness (verify, then fix if real):
-- **`vk_hook` / `vk_layer` `g_swaps`** — no synchronization on push/iterate and never pruned on
-  swapchain destroy (unbounded growth + stale-handle match). Add a guard + a `vkDestroySwapchainKHR`
-  prune.
 - **`mkb_hook` raw-input slot reuse** — a 64-slot ring can overwrite an event before the game reads
   the `WM_INPUT`. Verify; fix if real.
 - **Non-atomic cross-process diagnostic counters** — `present_calls`, `frames_dropped`,

hook/src/vk_hook.cpp

@@ -1,7 +1,9 @@
 #include "vk_hook.hpp"
 
+#include <algorithm>
 #include <atomic>
 #include <cstring>
+#include <mutex>
 #include <vector>
 
 #include <windows.h>
@@ -72,6 +74,16 @@ struct SwapInfo
 	std::vector<VkImage> images;
 };
 std::vector<SwapInfo> g_swaps;
+// g_swaps is pushed from the create-swapchain detour and read from the present detour, which can run
+// on different game threads (Vulkan external-sync is per-object, not global), and cleared from the
+// worker thread on removal. This mutex serializes all of that -- a push_back realloc must not race a
+// concurrent find_swap iteration. We don't hook vkDestroySwapchainKHR (failing to forward a destroy
+// could break the game); instead create de-dups by handle and an LRU cap bounds growth, so a game
+// that recreates its swapchain on every resize can't grow g_swaps without bound or match a recycled
+// handle's stale images. The active swapchain is always the most-recently created, so it's never
+// evicted.
+std::mutex g_swaps_mutex;
+constexpr std::size_t kMaxTrackedSwaps = 8; // engines use 1-3; headroom for transient resize overlap
 
 // --- the real vkGetInstanceProcAddr, via the inline hook's trampoline --------
 PFN_vkVoidFunction real_gipa(VkInstance inst, const char* name)
@@ -79,6 +91,8 @@ PFN_vkVoidFunction real_gipa(VkInstance inst, const char* name)
 	return g_hk_gipa.stdcall<PFN_vkVoidFunction>(inst, name);
 }
 
+// Caller must hold g_swaps_mutex; the returned pointer is only valid until the lock is released
+// (copy out what you need before unlocking, since another thread can push_back and reallocate).
 const SwapInfo* find_swap(VkSwapchainKHR sc)
 {
 	for (const SwapInfo& s : g_swaps)
@@ -107,12 +121,30 @@ VKAPI_ATTR VkResult VKAPI_CALL hk_vkQueuePresentKHR(VkQueue queue, const VkPrese
 	if (g_capture_enabled.load(std::memory_order_acquire) && g_device != VK_NULL_HANDLE &&
 	    pPresentInfo != nullptr && pPresentInfo->swapchainCount == 1)
 	{
-		const SwapInfo* s = find_swap(pPresentInfo->pSwapchains[0]);
+		// Copy the matched swapchain's fields out under the lock, then capture without holding it (so
-		if (s != nullptr && pPresentInfo->pImageIndices[0] < s->images.size())
+		// the GPU submit can't block a concurrent create, and the SwapInfo* can't dangle on a realloc).
+		VkImage image = VK_NULL_HANDLE;
+		VkFormat fmt = VK_FORMAT_UNDEFINED;
+		std::uint32_t w = 0, h = 0;
+		bool matched = false;
+		{
+			std::scoped_lock lock(g_swaps_mutex);
+			const SwapInfo* s = find_swap(pPresentInfo->pSwapchains[0]);
+			const std::uint32_t idx = pPresentInfo->pImageIndices[0];
+			if (s != nullptr && idx < s->images.size())
+			{
+				image = s->images[idx];
+				fmt = s->fmt;
+				w = s->w;
+				h = s->h;
+				matched = true;
+			}
+		}
+		if (matched)
 		{
 			VkSemaphore chained = VK_NULL_HANDLE;
-			if (g_cap.present(s->images[pPresentInfo->pImageIndices[0]], s->fmt, s->w, s->h,
+			if (g_cap.present(image, fmt, w, h, pPresentInfo->pWaitSemaphores,
-			                  pPresentInfo->pWaitSemaphores, pPresentInfo->waitSemaphoreCount, chained))
+			                  pPresentInfo->waitSemaphoreCount, chained))
 			{
 				// Replace the present's wait with our chained semaphore (our submit consumed the
 				// originals and signals this one), so the present still orders after rendering.
@@ -142,7 +174,19 @@ VKAPI_ATTR VkResult VKAPI_CALL hk_vkCreateSwapchainKHR(VkDevice device, const Vk
 		g_get_swapchain_images(device, *out, &n, nullptr);
 		info.images.resize(n);
 		g_get_swapchain_images(device, *out, &n, info.images.data());
-		g_swaps.push_back(std::move(info));
+		{
+			std::scoped_lock lock(g_swaps_mutex);
+			// De-dup a recycled handle value, then bound growth (drop the oldest; the just-created
+			// active swapchain is newest and stays).
+			g_swaps.erase(std::remove_if(g_swaps.begin(), g_swaps.end(),
+			                             [&](const SwapInfo& e) { return e.sc == info.sc; }),
+			              g_swaps.end());
+			g_swaps.push_back(std::move(info));
+			if (g_swaps.size() > kMaxTrackedSwaps)
+			{
+				g_swaps.erase(g_swaps.begin());
+			}
+		}
 		// (Don't log the VkSwapchainKHR handle: it's a uint64_t on x86, not a pointer.)
 		logf("vk: swapchain %ux%u fmt=%d images=%u", ci->imageExtent.width, ci->imageExtent.height,
 		     static_cast<int>(ci->imageFormat), n);
@@ -345,7 +389,10 @@ void remove_vk_hooks()
 	g_cap.shutdown(); // joins the reaper, drains the device, frees the read-back resources
 
 	hook_set_installed(g_id_present, false);
-	g_swaps.clear();
+	{
+		std::scoped_lock lock(g_swaps_mutex); // drained above, but keep all g_swaps access serialized
+		g_swaps.clear();
+	}
 	g_get_swapchain_images = nullptr;
 	g_device = VK_NULL_HANDLE;
 	g_instance = VK_NULL_HANDLE;

vk_layer/coop_vk_layer.cpp

@@ -15,10 +15,12 @@
 // The loader/layer interface structs (VkLayer*CreateInfo, VkNegotiateLayerInterface) live in
 // vk_layer.h, which Vulkan-Headers doesn't ship, so they're declared here to the stable
 // loader-interface-version-2 ABI.
+#include <algorithm>
 #include <atomic>
 #include <cstdarg>
 #include <cstdio>
 #include <cstring>
+#include <mutex>
 #include <string>
 #include <vector>
 
@@ -132,6 +134,12 @@ struct SwapInfo
 	std::vector<VkImage> images;
 };
 std::vector<SwapInfo> g_swaps;
+// Serialize all g_swaps access: create (push) and present (find) can run on different game threads,
+// and DestroyDevice clears it. We don't track swapchain destruction (forwarding a destroy wrong could
+// break the game); create de-dups by handle and an LRU cap bounds growth so repeated resize/recreate
+// can't grow g_swaps unbounded or match a recycled handle's stale images.
+std::mutex g_swaps_mutex;
+constexpr std::size_t kMaxTrackedSwaps = 8;
 
 bool eq(const char* a, const char* b)
 {
@@ -209,6 +217,8 @@ bool decide_active()
 	return _stricmp(self8, want) == 0;
 }
 
+// Caller must hold g_swaps_mutex; copy out what you need before unlocking (a concurrent push_back
+// can reallocate and dangle the returned pointer).
 const SwapInfo* find_swap(VkSwapchainKHR sc)
 {
 	for (const SwapInfo& s : g_swaps)
@@ -268,12 +278,28 @@ VKAPI_ATTR VkResult VKAPI_CALL layer_QueuePresentKHR(VkQueue queue, const VkPres
 	}
 	if (g_active && pi != nullptr && pi->swapchainCount == 1)
 	{
-		const SwapInfo* s = find_swap(pi->pSwapchains[0]);
+		// Copy the matched swapchain out under the lock, then capture without holding it.
-		if (s != nullptr && pi->pImageIndices[0] < s->images.size())
+		VkImage image = VK_NULL_HANDLE;
+		VkFormat fmt = VK_FORMAT_UNDEFINED;
+		std::uint32_t w = 0, h = 0;
+		bool matched = false;
+		{
+			std::scoped_lock lock(g_swaps_mutex);
+			const SwapInfo* s = find_swap(pi->pSwapchains[0]);
+			const std::uint32_t idx = pi->pImageIndices[0];
+			if (s != nullptr && idx < s->images.size())
+			{
+				image = s->images[idx];
+				fmt = s->fmt;
+				w = s->w;
+				h = s->h;
+				matched = true;
+			}
+		}
+		if (matched)
 		{
 			VkSemaphore chained = VK_NULL_HANDLE;
-			if (g_cap.present(s->images[pi->pImageIndices[0]], s->fmt, s->w, s->h, pi->pWaitSemaphores,
+			if (g_cap.present(image, fmt, w, h, pi->pWaitSemaphores, pi->waitSemaphoreCount, chained))
-			                  pi->waitSemaphoreCount, chained))
 			{
 				VkPresentInfoKHR p = *pi;
 				p.waitSemaphoreCount = 1;
@@ -305,7 +331,17 @@ VKAPI_ATTR VkResult VKAPI_CALL layer_CreateSwapchainKHR(VkDevice device, const V
 		g_get_swapchain_images(device, *out, &n, nullptr);
 		info.images.resize(n);
 		g_get_swapchain_images(device, *out, &n, info.images.data());
-		g_swaps.push_back(std::move(info));
+		{
+			std::scoped_lock lock(g_swaps_mutex);
+			g_swaps.erase(std::remove_if(g_swaps.begin(), g_swaps.end(),
+			                             [&](const SwapInfo& e) { return e.sc == info.sc; }),
+			              g_swaps.end());
+			g_swaps.push_back(std::move(info));
+			if (g_swaps.size() > kMaxTrackedSwaps)
+			{
+				g_swaps.erase(g_swaps.begin());
+			}
+		}
 	}
 	return r;
 }
@@ -435,7 +471,10 @@ VKAPI_ATTR void VKAPI_CALL layer_DestroyDevice(VkDevice device, const VkAllocati
 	if (g_active && device == g_device)
 	{
 		g_cap.shutdown(); // joins the reaper, drains the device, frees the read-back resources
-		g_swaps.clear();
+		{
+			std::scoped_lock lock(g_swaps_mutex);
+			g_swaps.clear();
+		}
 		g_device = VK_NULL_HANDLE;
 	}
 	destroy(device, a);