Hotplug docs: clarify thread-safety, safe-to-call list, device->next contract #790

Merged

Youw merged 5 commits from connection-doc-v2 into connection-callback

2026-07-13 16:59:37 -05:00

Youw commented

2026-04-24 09:16:24 -05:00

(Migrated from github.com)

Expands the hotplug API documentation to spell out the thread-safety contract and adjacent behaviour in line with the Multi-threading Notes wiki. Supersedes #784.

What changes

Doc-only changes to hidapi/hidapi.h:

hid_hotplug_callback_fn — explicit "Execution context", "What the callback may call", "device parameter", and "Return value" sections.
hid_hotplug_register_callback — new "Thread safety" section documenting that the hotplug register/deregister pair is thread-safe (the one deliberate exception to HIDAPI's general "not thread-safe" rule) and describing the lifetime of the internal hotplug thread.
hid_hotplug_deregister_callback — strengthened to document full thread-safety and the already-deregistered no-op behaviour.
Conventions & verified behaviour (second commit) — Since version 0.16.0 annotations on all new items; explicit @briefs (Doxygen autobrief is off); HID_API_HOTPLUG_ENUMERATE gated on the DEVICE_ARRIVED bit, handle assigned first so self-deregistration works; DEVICE_LEFT passes a fully-populated cached copy of the device info; invocation in registration order; handles are always positive (0 is a safe sentinel); implicit hid_init(); hid_exit() deregisters remaining callbacks; NetBSD returns -1; deregister of an unknown handle is safe and returns -1. Each of these was verified against all four backend implementations before being documented.
HID_API_HOTPLUG_ENUMERATE redefined as asynchronous (third commit, maintainer decision) — the initial pass for already-connected devices is now documented to be delivered on the same internal event context as live events, from a registration-time snapshot (each device reported exactly once — by the pass or by a live event), honouring the callback return value (non-zero stops the remainder of the pass and deregisters). The callback is thus never invoked on an application thread. All four backends currently deliver this pass synchronously on the registering thread and ignore the return value; feasibility was checked against each backend before adopting this (libusb/mac: existing injection channels; linux: pending-work check in the monitor loop; windows: needs a delivery vehicle, e.g. a thread-pool work item).

Why not #784's wording

#784's patch reads as a blanket "NOT safe to call hid_open/hid_open_path or any other global/non-device functions from within this callback." As @k1-801 pointed out in the review of that PR, this is overly strict: the issue is concurrency between the hotplug callback thread and any other thread that calls init/enumerate/open/close, not the callback itself. The new text follows the wiki's "may be used under conditions Y" style and enumerates the calls that are always safe from inside the callback.

Implied behaviour changes — tracked as follow-up issues

This PR documents the target semantics. A few minor implementation changes are required to bring the code in line with the docs; they are intentionally out of scope here (per maintainer decision — connection-callback is still WIP and a temporary doc/impl gap is acceptable):

device->next == NULL inside the callback — #792
HID_API_HOTPLUG_ENUMERATE honours the callback return value — #793
2a. HID_API_HOTPLUG_ENUMERATE delivers the initial pass asynchronously on the internal event context, from a registration-time snapshot, and before any live events for that callback (currently: synchronously on the registering thread in all four backends) — issue to be filed; supersedes the delivery half of item 8 below and naturally fixes #793 alongside
hid_hotplug_deregister_callback() on an unknown/already-deregistered handle returns 0 on Windows, while linux/libusb/mac (and now the docs) say -1 — issue to be filed
macOS hid_hotplug_register_callback() does not implicitly hid_init() (the other backends do), and hid_exit()'s hotplug cleanup is guarded by if (hid_mgr) — so hotplug-only usage on macOS leaks the hotplug thread and callback list at hid_exit() — issue to be filed
register/deregister failure paths (including argument validation) mostly do not set the error string promised via hid_error(NULL) — issue to be filed

Also captured as follow-ups — existing implementation bugs that predate this doc work:

macOS self-join deadlock in hid_internal_hotplug_cleanup — #794
hid_exit from within a hotplug callback should fail fast rather than deadlock — #795
windows/libusb subscribe to device notifications only after taking the ENUMERATE snapshot, so devices arriving in that window are missed entirely (linux/mac subscribe first and can at worst deliver a duplicate) — resolved by design by the snapshot-replay semantics of item 2a; fold into that issue

Not changing

No new public functions, enums, or flags.
No ABI or semantic break for correctly-written existing code.

Drafted with Claude Code.

Expands the hotplug API documentation to spell out the thread-safety contract and adjacent behaviour in line with the Multi-threading Notes wiki. Supersedes #784. ## What changes Doc-only changes to `hidapi/hidapi.h`: - **`hid_hotplug_callback_fn`** — explicit "Execution context", "What the callback may call", "device parameter", and "Return value" sections. - **`hid_hotplug_register_callback`** — new "Thread safety" section documenting that the hotplug register/deregister pair is thread-safe (the one deliberate exception to HIDAPI's general "not thread-safe" rule) and describing the lifetime of the internal hotplug thread. - **`hid_hotplug_deregister_callback`** — strengthened to document full thread-safety and the already-deregistered no-op behaviour. - **Conventions & verified behaviour** (second commit) — `Since version 0.16.0` annotations on all new items; explicit `@brief`s (Doxygen autobrief is off); `HID_API_HOTPLUG_ENUMERATE` gated on the `DEVICE_ARRIVED` bit, handle assigned first so self-deregistration works; `DEVICE_LEFT` passes a fully-populated cached copy of the device info; invocation in registration order; handles are always positive (0 is a safe sentinel); implicit `hid_init()`; `hid_exit()` deregisters remaining callbacks; NetBSD returns -1; deregister of an unknown handle is safe and returns -1. Each of these was verified against all four backend implementations before being documented. - **`HID_API_HOTPLUG_ENUMERATE` redefined as asynchronous** (third commit, maintainer decision) — the initial pass for already-connected devices is now documented to be delivered on the same internal event context as live events, from a registration-time snapshot (each device reported exactly once — by the pass or by a live event), honouring the callback return value (non-zero stops the remainder of the pass and deregisters). The callback is thus *never* invoked on an application thread. All four backends currently deliver this pass synchronously on the registering thread and ignore the return value; feasibility was checked against each backend before adopting this (libusb/mac: existing injection channels; linux: pending-work check in the monitor loop; windows: needs a delivery vehicle, e.g. a thread-pool work item). ## Why not #784's wording #784's patch reads as a blanket "NOT safe to call hid_open/hid_open_path or any other global/non-device functions from within this callback." As @k1-801 pointed out in the review of that PR, this is overly strict: the issue is concurrency between the hotplug callback thread and any other thread that calls init/enumerate/open/close, not the callback itself. The new text follows the wiki's "may be used under conditions Y" style and enumerates the calls that are always safe from inside the callback. ## Implied behaviour changes — tracked as follow-up issues This PR documents the **target** semantics. A few minor implementation changes are required to bring the code in line with the docs; they are intentionally out of scope here (per maintainer decision — connection-callback is still WIP and a temporary doc/impl gap is acceptable): 1. `device->next == NULL` inside the callback — #792 2. `HID_API_HOTPLUG_ENUMERATE` honours the callback return value — #793 2a. `HID_API_HOTPLUG_ENUMERATE` delivers the initial pass asynchronously on the internal event context, from a registration-time snapshot, and before any live events for that callback (currently: synchronously on the registering thread in all four backends) — issue to be filed; supersedes the delivery half of item 8 below and naturally fixes #793 alongside 3. `hid_hotplug_deregister_callback()` on an unknown/already-deregistered handle returns **0** on Windows, while linux/libusb/mac (and now the docs) say **-1** — issue to be filed 4. macOS `hid_hotplug_register_callback()` does not implicitly `hid_init()` (the other backends do), and `hid_exit()`'s hotplug cleanup is guarded by `if (hid_mgr)` — so hotplug-only usage on macOS leaks the hotplug thread and callback list at `hid_exit()` — issue to be filed 5. register/deregister failure paths (including argument validation) mostly do not set the error string promised via `hid_error(NULL)` — issue to be filed Also captured as follow-ups — existing implementation bugs that predate this doc work: 6. macOS self-join deadlock in `hid_internal_hotplug_cleanup` — #794 7. `hid_exit` from within a hotplug callback should fail fast rather than deadlock — #795 8. windows/libusb subscribe to device notifications only *after* taking the `ENUMERATE` snapshot, so devices arriving in that window are missed entirely (linux/mac subscribe first and can at worst deliver a duplicate) — resolved by design by the snapshot-replay semantics of item 2a; fold into that issue ## Not changing - No new public functions, enums, or flags. - No ABI or semantic break for correctly-written existing code. --- _Drafted with Claude Code._