windows: deliver hotplug ENUMERATE pass asynchronously on the event context

[Youw]

Implements the hotplug contract documented via #790 for the windows backend (windows/hid.c only).

What changed

• Asynchronous ENUMERATE pass. hid_hotplug_register_callback no longer runs the HID_API_HOTPLUG_ENUMERATE pass synchronously on the calling thread. Instead it takes a deep-copied snapshot of the matching entries of the cached device list (under the hotplug critical section, only when the flag is set and events includes DEVICE_ARRIVED) into a per-callback replay list, and submits a threadpool work item (CreateThreadpoolWork/SubmitThreadpoolWork) that delivers it on the same critical-section-serialized context that delivers live CM_Register_Notification events. The pass never fires from within register itself and never on an application thread.
• Exactly once, pass-before-live. Live dispatch flushes a callback's pending replay before delivering any live event to it, and is bounded to the callbacks present at event time, so a callback registered from inside another callback picks the in-flight device up from its snapshot rather than from the live loop (previously it could see it twice). device->next is now NULL for every invocation, including the synthetic ones (the old sync pass handed out devices still linked into the cached list).
• Callback return value honored during the pass (Hotplug: HID_API_HOTPLUG_ENUMERATE should honour callback return value #793, windows): a non-zero return stops the remainder of the pass, frees the undelivered snapshot and deregisters the callback.
• Deregister return value: 0 only when the handle was found and deregistered; -1 for unknown, stale or already-self-deregistered handles (safe no-op).
• Error strings: all register/deregister failure paths (including argument validation) set the global error retrievable via hid_error(NULL); *callback_handle is zeroed on any registration failure.
• Teardown safety: CM_Unregister_Notification is documented to deadlock when called from its own notification callback, and it also deadlocks if called while holding a lock the notification callback takes. The teardown now detaches the notification handle under the critical section and unregisters outside it; when the last callback removes itself from inside the notification callback, the actual teardown is deferred to the threadpool work item. hid_exit frees undelivered snapshots, waits for in-flight work items (WaitForThreadpoolWorkCallbacks) before deleting the critical section, and sweeps anything a last-gasp notification appended to the device cache.

Design notes

• The work item is a single reusable PTP_WORK created with the first callback registration and closed by hid_exit. I used CreateThreadpoolWork rather than TrySubmitThreadpoolCallback because teardown needs WaitForThreadpoolWorkCallbacks to guarantee no work item can touch the critical section after hid_exit deletes it; the fire-and-forget API offers no way to wait.
• Replay flushing is idempotent and runs under the critical section with mutex_in_use set: whichever of the work item or a live event acquires the lock first delivers the pass, the other finds replay == NULL.
• Registration reuses an already-registered CM notification when a deferred teardown has not run yet (and refreshes the cached device list), instead of failing as the old code did.
• The threadpool API needs Vista-level declarations, so the file now raises _WIN32_WINNT to 0x0600 when it is set lower (the dynamically-resolved CM notification API already requires Windows 8 at runtime).

Addresses #793 for the windows backend.

Assisted-by: claude-code:claude-fable-5

Review round 1

Reviewed independently by two reviewers against the #790 contract; all confirmed findings are addressed in a3350c4:

• Enumerate-before-subscribe blind window (blocker) — the CM notification is now armed before the registration-time snapshot, and arrivals are deduplicated by path in the notification callback, so a device connecting between the two is caught by the notification and absorbed by the dedupe instead of being reported by neither.
• Detached-but-live notification vs a fresh registration, and cache refresh during a deferred teardown — arming a new notification now waits for in-flight CM_Unregister_Notification calls to complete (pending counter + condition variable), and the device cache is rebuilt only when a notification is actually being armed. Both windows could previously deliver one connection twice.
• hid_exit racing a concurrent deregister — the same quiescence gate is awaited before destroying state (previously it could DeleteCriticalSection / CloseThreadpoolWork while a detached notification was still live at the OS). A failed CM_Unregister_Notification now poisons the epoch: hid_exit reports the failure and deliberately leaks the critical section, work item and DLL handles rather than let a live notification touch destroyed state.
• Initialization and error-reporting races — the machinery bootstrap is guarded by a statically-initialized SRWLOCK (two racing first registrations could both initialize the critical section), and all mutations of the global error string are serialized by another SRWLOCK (concurrent failures in the documented-thread-safe register/deregister could double-free it).
• Enumeration failure vs empty system — the two are now distinguished (registration fails on genuine failure), and the stale "No HID devices found" error is cleared on successful registration.
• Handle exhaustion — registration now fails with an error instead of signed-overflowing and wrapping onto live handles.
• C++ cleanliness — { 0 } struct initializers replaced with memset for -Wmissing-field-initializers under GNU C++.

Review round 2

Reviewed independently by two further reviewers; all confirmed findings are addressed in 1f621c1:

• Critical section lifetime (blocker) — hid_exit deleted the critical section with no lock held across the teardown, while a registration sleeping on a pending unregistration, a concurrent (contract-legal) deregistration, or hid_internal_hotplug_init could still be about to enter it. The critical section and the quiescence event are now created once and never destroyed (consistent with the leaked-notification path, which already had to keep them), and an explicit exiting state, set under the lock, makes registration and deregistration fail rather than arm a notification behind a teardown. mutex_ready is only ever read under the bootstrap lock or under the critical section itself.
• hid_exit unloading the DLLs under a concurrent registration — found by the new stress test. hid_hotplug_register_callback is thread-safe and initializes the library implicitly, so it could be calling CM_Register_Notification through a function pointer while hid_exit was FreeLibrary-ing cfgmgr32.dll. exiting is now held across the unload; every other call through those pointers is either made under the critical section or accounted for by the pending-unregistration counter that the teardown waits out.
• Global error string written from an internal thread — hid_error(NULL) returns the raw string pointer with no lock, so the threadpool work item reporting a failed CM_Unregister_Notification could free() it under an application thread: a use-after-free the application cannot serialize away. HIDAPI's internal event context no longer writes the global error at all; the failure is recorded in internal state and reported by the next hid_exit. The writer-side lock (round 1) is kept for the application-thread writers.
• Poisoned epoch was permanent and re-armable — the failure is now scoped to the hid_exit that reports it (a single failed unregistration no longer makes every later hid_exit return -1), while the safety consequences stay for the process: the state a live notification can reach is never destroyed, the DLLs it calls into are never unloaded, the module that contains the callback is pinned (GetModuleHandleEx), and no second notification is ever armed next to it — registration fails with an explicit error, because two live registrations would deliver every event twice.
• Path dedupe was permanent — an interface path is reused when a device is replugged into the same port, so deduplicating every arrival against the cache could swallow a genuine new connection. The dedupe now covers only the window it exists for (the overlap between arming the notification and taking the registration-time snapshot): the enumerated paths are recorded, each swallows at most one arrival, and a path is dropped as soon as its device leaves. Outside that window an arrival for a cached path is a new connection and replaces the stale record instead of being shadowed by it.
• OOM handling in the enumeration — a per-device allocation failure was treated as a successful (but silently short) enumeration, so a registration could succeed with devices missing from the pass; it now goes through the failure channel. A hid_device_info is never handed out half-built, so the hotplug cache can no longer hold a record with a NULL path — which the removal walk dereferenced — and the bus-specific fixups (hid_internal_get_usb_info/_get_ble_info, which wcslen() the strings) can no longer read a NULL.
• _WIN32_WINNT force-bump removed (supersedes the round-1 design note) — it turned the threadpool, SRWLOCK and condition-variable calls into load-time Vista-only imports of hidapi.dll for every user, including those that never touch hotplug, and it contradicted the file's LoadLibrary/GetProcAddress discipline. The four threadpool entry points are now resolved from kernel32 like everything else (hotplug registration fails with an error if they are unavailable), and the two Vista-only synchronisation primitives were replaced with equivalents that need no initialization (the quiescence wait is a manual-reset event; the two tiny writer locks are interlocked locks). dumpbin /imports on the resulting DLL shows no *Threadpool*, *SRWLock* or *ConditionVariable* symbols.

Verified with the CI recipes (MSVC C and C++, /W4 /WX, ASAN), the repo's ctest suite, and a throwaway white-box contract test (synthetic CM notifications + allocation fault injection) covering the async pass semantics, non-zero return, self-deregistration, register-from-callback, stale handles, hid_exit with a pending replay, re-init, argument validation, hid_exit racing register/deregister storms, replug-on-the-same-path, NULL-path cache records and 400 OOM injection points — 10x stress runs, all green.

Review round 3

A delta review of 1f621c1 (read and executed: MSVC C and C++20 /W4 /WX builds, dumpbin /imports, a DLL load/unload handle-leak harness, and a 6-thread register/deregister storm racing hid_exit) confirmed the round-2 synchronization model holds, and found one newly-introduced blocker plus several minors; all are addressed in 0ddecf2:

• hid_exit leaked a kernel event and a critical section for every user (blocker, introduced by round 2) — round 2 made hid_exit bootstrap the hotplug machinery just to raise the exiting flag, and the never-destroy policy then guaranteed the event handle and the critical section were never freed: a plain hid_init/hid_enumerate/hid_exit program that never touches hotplug leaked one handle per load/use/unload cycle (measured: +100 handles over 100 cycles), contradicting hid_exit's documented purpose. The flag now lives under the statically-initialized bootstrap spinlock — which costs no OS object — so hid_exit can raise it without creating anything and return early when the machinery never existed. On top of that, every public hotplug call is now counted into the machinery under that same lock, which supplies the proof round 2 lacked: once exiting cuts off new entries, every notification is unregistered, the work item is drained and closed, and the user count is zero, nothing can be inside — or blocked on — the critical section, so the clean path of hid_exit now destroys the event and critical section it created. The leaked-notification path keeps everything alive forever, exactly as before. Measured after the fix: 0 handle growth over 100 cycles, with and without hotplug use in the loop (was +100 in both).
• Stale pending-arrival records swallowed genuine re-arrivals — the registration-time dedupe records suppressed the one in-flight duplicate they exist for, but a record whose duplicate never came outlived that window; if the device's removal notification was then missed, its genuine re-arrival was consumed by its own stale record, and the stale-record recovery could never run for exactly the devices it targets. The list now expires (10 s, far beyond any notification delivery latency): the failure modes are asymmetric in the right direction — an over-aged duplicate would produce a spurious left+arrived pair instead of a silently dropped connection.
• Stale-record replacement owed a DEVICE_LEFT — replacing a stale cache record on a same-path arrival reported the new connection but silently discarded the previous one, although the header promises a "left" event for every matching device that disconnects while a callback is registered. The live dispatch loop is factored out (hid_internal_hotplug_dispatch) and the stale record is now delivered as a synthetic DEVICE_LEFT before the new device is cached, preserving exactly-once for callbacks registered from within a callback.
• Spinlock discipline — the global error message is now built and freed outside the internal spinlock (only the pointer swap is guarded; previously FormatMessageW plus free/calloc ran under it), and the acquire loop escalates from Sleep(0) to Sleep(1) after a few attempts: Sleep(0) yields only to equal-or-higher-priority threads, so spinning against a lower-priority holder on a single CPU made no progress until the balance-set manager intervened.
• One malformed interface path failed the whole enumeration as an OOM — hid_internal_UTF16toUTF8 returns NULL both for invalid UTF-16 (e.g. an unpaired surrogate) and on allocation failure, and round 2 escalated both to a full-enumeration failure reported as "Failed to allocate memory". The two are now distinguished: an unrepresentable device is skipped (consistently with the hotplug notifications, which can neither report nor cache it), and only genuine allocation failures abort the pass.
• Also: corrected the comment on the global error lock (a user callback calling the public hotplug API from the event context does write the global error; the header's serialization requirement covers it), removed a dead failed term the sticky notification_leaked flag always implies, and documented why the registration wait loop may release the critical section unconditionally (it can never be held recursively there).

Verified with the CI recipes (MSVC C and C++20, /W4 /WX, ASAN, plus the HIDAPI_USE_DDK compile), dumpbin /imports (still no *Threadpool*/*SRWLock*/*ConditionVariable* imports; the only new kernel32 import is GetTickCount), the handle-leak harness above, and the white-box contract test extended with the new cases (pending-arrival expiry, missed-removal recovery delivering left+arrived, invalid-UTF-16 vs OOM) — including 3x runs of the 8-second 6-thread hid_exit race, all green.

---

Final state (after review) — supersedes parts of the description above

The pending-arrival / wall-clock-expiry / missed-removal-recovery scheme described earlier was removed. The final design: arrival de-duplication and removal are an allocation-free live-cache path comparison (a hand-rolled, surrogate-validated UTF-16↔UTF-8 compare), so an OOM on the removal path can never leave the cache stale; the four threadpool APIs are resolved dynamically via GetProcAddress (keeping the DLL's load-time OS floor at XP — no static Vista-only imports); a user-count enter/leave gate lets hid_exit() prove no thread is inside or blocked on the critical section before destroying it (0 leaked handles for programs that never touch hotplug); and internal-event-context writes to the global error string are suppressed via a thread-local flag.

Verification. Every fix commit on this branch was adversarially delta-verified by an independent reviewer (Codex on a separate plan, or a fresh model) tasked to find what the fix itself broke — a loop that caught a defect in nearly every fix commit. A per-backend white-box harness injects fake device events into the real implementation and runs under ASan/UBSan/LSan and TSan (TSan proven live). The hotplug-integration branch — all four final backends merged with the test suite (#826) — is CI-green on every platform, with the hotplug contract tests executing against this backend, not skipped.

Addresses the Windows register_callback error-handling review (#785) and implements device->next == NULL (#791, #792) and the ENUMERATE return-value (#793) contract for winapi. Part of hotplug support (#238); contract documented in #790.

Assisted-by: claude-code:claude-opus-4-8