test: the recorder exit check catches the old destructor on llvm-mingw too #2240

Merged

klimovich008 merged 1 commit from fix/recorder-exit-sentinel-order into main

2026-11-11 19:48:19 +00:00

klimovich008 commented

2026-11-11 10:26:46 +00:00

(Migrated from github.com)

tests/test_recorder_sync.py fails on Windows with llvm-mingw: its exit check's negative control (the old recorder destructor) passes there, so the runner reports "FAIL: the exit test passes the old destructor" (reported by Arkex-ai on #1473). The control was not discriminating on that toolchain; nothing in the game changed.

Cause

The fixture's sentinel must be destroyed after the recorder's global so that its line proves every DLL destructor ran. The recorder's global (g_rec) is an inline variable, and inline variables' dynamic initialization is only partially ordered against a file's own statics. With llvm-mingw 20261104, g_rec was made first, so the sentinel printed its line before the old destructor ended the process.

Static linking (as hookshot_vk.dll links) made no difference; a minimal DLL with a mutex held by a thread killed at ExitProcess shows llvm-mingw's libc++ ending the process when a destructor takes that mutex, as the fixture expects.

Change

tests/recorder_exit_test.cpp: the sentinel gets __attribute__((init_priority(101))), so it is constructed first and destroyed last on every compiler that builds the test. The header comment now says which runtimes hang and which end the process.

Tested

Windows 11, python -I -X utf8 -B tests/test_recorder_sync.py --cxx <cxx> --headers <Vulkan-Headers/include>:

Compiler	Current code	Old destructor (broken variant)
llvm-mingw 20261104 clang++	pass	caught: ended on the spot, sentinel missing
MSYS2 ucrt64 clang++ 23.1.5	pass	caught: timeout
MSYS2 ucrt64 g++ 16.2	pass	caught: timeout
clang 23.1.5, MSVC target	pass	caught: ended on the spot

Before the change, llvm-mingw reproduced the reported failure (NOT CAUGHT, exit code 0 with the sentinel line).

Not covered

Only the test fixture changes; Linux skips this check (Windows-only, as before).

🤖 Generated with Claude Code

`tests/test_recorder_sync.py` fails on Windows with llvm-mingw: its exit check's negative control (the old recorder destructor) passes there, so the runner reports "FAIL: the exit test passes the old destructor" (reported by Arkex-ai on #1473). The control was not discriminating on that toolchain; nothing in the game changed. ## Cause The fixture's sentinel must be destroyed after the recorder's global so that its line proves every DLL destructor ran. The recorder's global (`g_rec`) is an inline variable, and inline variables' dynamic initialization is only partially ordered against a file's own statics. With llvm-mingw 20261104, `g_rec` was made first, so the sentinel printed its line before the old destructor ended the process. Static linking (as `hookshot_vk.dll` links) made no difference; a minimal DLL with a mutex held by a thread killed at `ExitProcess` shows llvm-mingw's libc++ ending the process when a destructor takes that mutex, as the fixture expects. ## Change - `tests/recorder_exit_test.cpp`: the sentinel gets `__attribute__((init_priority(101)))`, so it is constructed first and destroyed last on every compiler that builds the test. The header comment now says which runtimes hang and which end the process. ## Tested Windows 11, `python -I -X utf8 -B tests/test_recorder_sync.py --cxx <cxx> --headers <Vulkan-Headers/include>`: | Compiler | Current code | Old destructor (broken variant) | |---|---|---| | llvm-mingw 20261104 clang++ | pass | caught: ended on the spot, sentinel missing | | MSYS2 ucrt64 clang++ 23.1.5 | pass | caught: timeout | | MSYS2 ucrt64 g++ 16.2 | pass | caught: timeout | | clang 23.1.5, MSVC target | pass | caught: ended on the spot | Before the change, llvm-mingw reproduced the reported failure (NOT CAUGHT, exit code 0 with the sentinel line). ## Not covered Only the test fixture changes; Linux skips this check (Windows-only, as before). 🤖 Generated with [Claude Code](https://claude.com/claude-code)

hckrman101 (Migrated from github.com) approved these changes

2026-11-11 19:47:24 +00:00

hckrman101 (Migrated from github.com) left a comment

The test is still declared via RUN_ALL_ARGS in test_recorder_sync.py, and the exit check still skips off Windows, so Linux is unaffected. CI is green on Linux and Windows (windows-2022 source checks, llvm-mingw Vulkan back end). The four-compiler table in the description (pass on current code, old destructor caught) covers the real fix; I did not run it myself.

Nit: the sentinel's comment now explains the ordering well; no change needed.

Looks good. The diagnosis fits: an inline variable's dynamic init is only partially ordered against the file's own statics, so llvm-mingw could build g_rec first and the sentinel then printed before the old destructor killed the process. init_priority(101) makes the sentinel first-constructed/last-destroyed, which is what the comment already promised, and 101 is outside the reserved 0-100 range. It is a plain attribute that clang (both targets) and GCC accept, so no portability issue, and the change is test-only with no timing or thread logic added (the 20 s watchdog and the negative control in tests/test_recorder_sync.py are untouched). The test is still declared via RUN_ALL_ARGS in test_recorder_sync.py, and the exit check still skips off Windows, so Linux is unaffected. CI is green on Linux and Windows (windows-2022 source checks, llvm-mingw Vulkan back end). The four-compiler table in the description (pass on current code, old destructor caught) covers the real fix; I did not run it myself. Nit: the sentinel's comment now explains the ordering well; no change needed.