Guard agent/probe threads against zeroed g_vms[] slots

When a VM or template at a lower index is removed (deleted, or template
finishes sysprep), the g_vms[] array is compacted: entries shift down
and the now-vacated last slot is zeroed. Any background thread that
cached its VM pointer as &g_vms[idx] before the shift is left pointing
at the wrong slot (either zeroed or holding another VM's data).

Concrete repro: create a template, then create a regular VM while the
template is still building. The regular VM's agent thread holds
&g_vms[1]. When the template at g_vms[0] finishes and gets removed,
the regular VM's data shifts to g_vms[0] and &g_vms[1] is zeroed. The
agent thread now loops every few seconds calling
hcs_find_runtime_id(L"", ...) -- which logs the symptom the user saw:

  HCS enumerate: [{...running VM...}]
  hcs_find_runtime_id: VM "" not found in enumeration

Surgical fix: the agent loop and IDD-probe loop both bail out cleanly
when vm->name[0] is empty, which is the post-zero state. Stops the
log spam and the wasted IOCTLs.

This is a defensive guard, not a root-cause fix. The deeper issue is
that g_vms[] uses unstable indices: any cached VmInstance* can be
invalidated by compaction. A proper fix replaces shift+truncate with
stable slots marked as deleted, which is a larger refactor.
This commit is contained in:
James Stringer committed 2026-05-17 11:46:19 -04:00
1 parent 423b9a75a4
commit dc3fefa3f5
2 files changed
+14 -3

No files matched your search

+8 -2
View File
@@ -163,7 +163,13 @@ static DWORD WINAPI idd_probe_thread_proc(LPVOID param)
asb_log(L"IDD probe: starting for \"%s\"", vm->name);
while (!vm->idd_probe_stop && vm->running && !vm->install_complete) {
/* `vm` points into g_vms[] by index. If a lower-index entry is removed
(e.g. a template finishes sysprep), our slot's contents shift away
and our pointer's memory gets zeroed. The vm->running check usually
catches that (zeroed slot has running=FALSE), but if the slot got
overwritten with another VM's data instead of zeroed, the name
check ensures we don't silently start probing the wrong VM. */
while (!vm->idd_probe_stop && vm->running && !vm->install_complete && vm->name[0] != L'\0') {
/* Try to connect to VDD frame service */
static const GUID zero_guid = {0};
GUID runtime_id = vm->runtime_id;
@@ -213,7 +219,7 @@ static DWORD WINAPI idd_probe_thread_proc(LPVOID param)
/* Connection succeeded - VDD is accepting frames */
closesocket(s);
if (!vm->idd_probe_stop && vm->running && !vm->install_complete) {
if (!vm->idd_probe_stop && vm->running && !vm->install_complete && vm->name[0] != L'\0') {
asb_log(L"IDD probe: VDD ready for \"%s\", opening display", vm->name);
if (g_idd_probe_hwnd)
PostMessageW(g_idd_probe_hwnd, WM_VM_IDD_READY, 0, (LPARAM)vm);
+6 -1
View File
@@ -317,7 +317,12 @@ static DWORD WINAPI agent_thread_proc(LPVOID param)
AgentConn *conn = (AgentConn *)param;
VmInstance *vm = conn->vm;
while (!conn->stop) {
/* `vm` points into g_vms[] by index. The array is compacted (shifted
down + last slot zeroed) when a VM/template at a lower index is
removed - which means our slot's contents can be silently zeroed
under us. Detect that via the now-empty name and bail out cleanly,
otherwise we loop forever calling hcs_find_runtime_id(""). */
while (!conn->stop && vm->name[0] != L'\0') {
char buf[256];
int n;
SOCKET s;