Skip to content

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

Merged
Youw merged 5 commits into
connection-callbackfrom
connection-doc-v2
Jul 13, 2026
Merged

Hotplug docs: clarify thread-safety, safe-to-call list, device->next contract#790
Youw merged 5 commits into
connection-callbackfrom
connection-doc-v2

Conversation

@Youw

@Youw Youw commented Apr 24, 2026

Copy link
Copy Markdown
Member

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):

  1. device->next == NULL inside the callback — Hotplug: device->next must be NULL inside callback #792
  2. HID_API_HOTPLUG_ENUMERATE honours the callback return value — Hotplug: HID_API_HOTPLUG_ENUMERATE should honour 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 Hotplug: HID_API_HOTPLUG_ENUMERATE should honour callback return value #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:

  1. macOS self-join deadlock in hid_internal_hotplug_cleanupHotplug (macOS): self-join deadlock in hid_internal_hotplug_cleanup #794
  2. hid_exit from within a hotplug callback should fail fast rather than deadlock — Hotplug: hid_exit() from inside callback should fail fast, not deadlock #795
  3. 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.

Expand the doc comments on hid_hotplug_callback_fn,
hid_hotplug_register_callback, and hid_hotplug_deregister_callback
to make the hotplug API's thread-safety contract explicit:

- The hotplug API is thread-safe; this is a documented exception
  to HIDAPI's general "not thread-safe" rule.
- The callback runs on an internal HIDAPI thread that is distinct
  from the application's threads, and holds an internal mutex for
  the duration of each call.
- Enumerate safe-to-call functions from within the callback
  (hid_hotplug_register/deregister_callback, hid_error(dev)) and
  explain why others require application-level serialisation,
  matching the wording style in the Multi-threading Notes wiki.
- Spell out the lifetime of the device pointer, the string
  ownership rules, and the requirement that device->next is
  always NULL inside the callback.
- Document hid_exit() as UB from within the callback (self-join).
- Define the return-value semantics consistently for both live
  events and the synthetic "arrived" events delivered during
  HID_API_HOTPLUG_ENUMERATE.

Supersedes PR #784. Behaviour changes implied by this doc
(device->next=NULL, ENUMERATE honoring return value) are tracked
as follow-up issues.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Youw added 4 commits July 13, 2026 18:45
…havior

Add "Since version 0.16.0" annotations and missing @brief's; document
the ENUMERATE synchronous-delivery semantics, DEVICE_LEFT cached device
info, invocation order, handle positivity, implicit hid_init and
hid_exit cleanup; fix the deregister return-value contract (-1 for
unknown handles) and minor wording.

Where the documented (target) semantics diverge from a backend's
current behavior, the gap is tracked as a follow-up, same as the rest
of this PR.

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

Adopt the async design for HID_API_HOTPLUG_ENUMERATE: the initial pass
for already-connected devices is delivered on the same internal event
context as live events, from a registration-time snapshot (exactly-once,
before any live events for that callback), honoring the callback return
value (early stop + deregister). The callback is thus never invoked on
an application thread.

Also tighten the contract per review: deregistration post-condition
(safe to free user_data after return), handle non-reuse, argument
validation, out-param on failure, DEVICE_LEFT ordering and coverage,
and replace markdown headings with @Par (Doxygen MARKDOWN_SUPPORT is
off, so ## rendered as literal text).

Assisted-by: claude-code:claude-fable-5
Registering/deregistering from within a callback re-acquires the mutex
on the same (event context) thread, so it cannot deadlock; only calls
from other threads block until the callback returns. State this
explicitly instead of leaving it implied by the word "other".

Assisted-by: claude-code:claude-fable-5
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation hotplug Related to hotplug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants