fix: resolve hook_enable() startup deadlock and lost wakeup

[alxndalexeev]

Summary

hook_enable() has a startup race that can permanently deadlock the calling thread. For Electron and other GUI consumers, the calling thread is the process main thread, so the symptom is an app that hangs at launch, paints nothing, and ignores SIGTERM — only a force-quit clears it.

There are two independent defects in the same handshake. Both are reproduced deterministically below, and both are fixed here.

Defect 1 — uv_cond_wait() with no predicate loop

uv_cond_wait(&hook_control_cond, &hook_control_mutex);

if (uv_mutex_trylock(&hook_running_mutex) == 0) {
  // "Lock Successful; The hook is not running but the hook_control_cond
  //  was signaled! This indicates that there was a startup problem!"
  uv_thread_join(&hook_thread);
  ...

uv_cond_wait() is permitted to wake spuriously. When it does, the hook thread has not reached EVENT_HOOK_ENABLED yet, so nothing holds hook_running_mutex and the trylock succeeds. That is read as "startup problem", and uv_thread_join() is then called while hook_control_mutex is still held (locked on entry, not released until the end of the function) and hook_running_mutex has just been taken.

Those are exactly the two mutexes the hook thread needs to complete EVENT_HOOK_ENABLED:

case EVENT_HOOK_ENABLED:
  uv_mutex_lock(&hook_running_mutex);   // blocks: hook_enable() holds it
  uv_mutex_lock(&hook_control_mutex);   // also held by hook_enable()

The hook thread blocks; hook_enable() waits for it in uv_thread_join(). Neither can proceed.

The root cause is using a mutex as a state flag. trylock() answers "is this held right now", which is not the question being asked ("has the hook finished starting"). The two answers diverge on precisely the interleaving that deadlocks.

Defect 2 — cross-thread unlock and lost wakeup

hook_thread_status = hook_run();

uv_cond_signal(&hook_control_cond);
uv_mutex_unlock(&hook_control_mutex);   // never locked on THIS thread

On the normal path this pairs with the uv_mutex_lock(&hook_control_mutex) in EVENT_HOOK_DISABLED. But if hook_run() fails before dispatching EVENT_HOOK_ENABLED — a denied macOS Accessibility/Input Monitoring grant, no X display, an event tap that cannot be created — that lock never happened, and this unlocks a mutex the thread does not own. That is undefined behaviour.

The signal is also sent without the mutex held, so it can be delivered before hook_enable() starts waiting. Condition variables do not queue signals, so it is simply lost and the caller waits forever.

The fix

Replace the mutex-as-flag with an explicit hook_start_state guarded by hook_control_mutex, wait on it in a predicate loop, and have hook_thread_proc() acquire the mutex it signals under.

hook_running_mutex then carries no information the state variable does not, so it is removed — along with the EVENT_HOOK_DISABLED branch that existed only to release it. The failure-path uv_thread_join() now runs with the control mutex released, so it cannot block on anything the hook thread might still want.

Behaviour is otherwise unchanged: hook_enable() still returns UIOHOOK_SUCCESS once the hook reports itself enabled, and still returns the thread's status when startup fails.

One incidental removal: the logger_proc(LOG_LEVEL_DEBUG, ...) call in hook_enable(). logger_proc only handles LOG_LEVEL_WARN and LOG_LEVEL_ERROR, so it was already a no-op. Happy to restore it if you'd rather keep it.

Verification

Real module, macOS 26.6 / arm64 / Node 26, rebuilt from this branch — npx node-gyp rebuild compiles clean (no new warnings from uiohook_worker.c):

• start() returned in 61 ms, 15 real keyboard/mouse events captured, stop() returned in 1 ms.
• 8 consecutive start()/stop() cycles, all clean, no hang or crash.

Deterministic repro of both defects. The race is timing-dependent in the wild, so this harness extracts only the synchronization algorithm — no libuiohook, no permissions, no real event tap — and injects the spurious wakeup explicitly, since uv_cond_wait is allowed to produce one. hook_run() is stubbed to do what the real one does on the path that matters.

Defect 1 (spurious wakeup):        old: 3/3 deadlock (killed at 10s)   new: 5/5 exit 0
Defect 2 (hook_run fails early):   old: 3/3 hang, wakeup lost          new: 3/3 report failure

repro.c — cc repro.c -o repro -luv -lpthread, then ./repro old vs ./repro new

// Deterministic repro of the uiohook-napi hook_enable() startup deadlock.
//
// Extracts ONLY the synchronization algorithm from src/lib/uiohook_worker.c so
// it runs anywhere, with no libuiohook, no permissions and no real event tap.
// hook_run() is stubbed to do what the real one does on the path that matters:
// dispatch EVENT_HOOK_ENABLED, then block until asked to stop.
//
// The spurious wakeup is injected explicitly (a helper thread signals the
// condvar shortly after the hook thread is created). uv_cond_wait is permitted
// to return spuriously, so this is a legal execution the real code must survive.
//
// Build: cc repro.c -o repro -luv -lpthread
// Run:   ./repro old   -> hangs (deadlock)
//        ./repro new   -> exits 0

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <uv.h>

#define UIOHOOK_SUCCESS 0
#define UIOHOOK_FAILURE 1
#define UIOHOOK_ERROR_THREAD_CREATE 0x10

static uv_thread_t hook_thread;
static int hook_thread_status;
static uv_mutex_t hook_running_mutex;   // OLD only
static uv_mutex_t hook_control_mutex;
static uv_cond_t  hook_control_cond;

typedef enum { HOOK_START_PENDING = 0, HOOK_START_RUNNING, HOOK_START_FAILED } hook_start_state;
static hook_start_state hook_state;     // NEW only

static int use_new;
static uv_mutex_t stop_mutex;
static uv_cond_t  stop_cond;
static int stop_requested;

// ---- stub for libuiohook's hook_run(): dispatch ENABLED, then block ---------
static void dispatch_hook_enabled(void) {
  // Real EVENT_HOOK_ENABLED delivery is not instantaneous; this widens the
  // window the injected spurious wakeup has to land in. It does not create the
  // bug, it just makes an existing race hit every time instead of rarely.
  usleep(50 * 1000);

  if (use_new) {
    uv_mutex_lock(&hook_control_mutex);
    hook_state = HOOK_START_RUNNING;
    uv_cond_signal(&hook_control_cond);
    uv_mutex_unlock(&hook_control_mutex);
  } else {
    uv_mutex_lock(&hook_running_mutex);
    uv_mutex_lock(&hook_control_mutex);
    uv_cond_signal(&hook_control_cond);
    uv_mutex_unlock(&hook_control_mutex);
  }
}

static int hook_run(void) {
  dispatch_hook_enabled();
  uv_mutex_lock(&stop_mutex);
  while (!stop_requested) uv_cond_wait(&stop_cond, &stop_mutex);
  uv_mutex_unlock(&stop_mutex);
  return UIOHOOK_SUCCESS;
}

static void hook_thread_proc(void* arg) {
  (void)arg;
  hook_thread_status = hook_run();
  if (use_new) {
    uv_mutex_lock(&hook_control_mutex);
    if (hook_state == HOOK_START_PENDING) hook_state = HOOK_START_FAILED;
    uv_cond_signal(&hook_control_cond);
    uv_mutex_unlock(&hook_control_mutex);
  } else {
    uv_cond_signal(&hook_control_cond);
    uv_mutex_unlock(&hook_control_mutex);
  }
}

// Injects the spurious wakeup uv_cond_wait is allowed to produce.
static void spurious_proc(void* arg) {
  (void)arg;
  usleep(5 * 1000);
  uv_mutex_lock(&hook_control_mutex);
  uv_cond_signal(&hook_control_cond);
  uv_mutex_unlock(&hook_control_mutex);
}

static int hook_enable_old(void) {
  uv_mutex_lock(&hook_control_mutex);
  int status = UIOHOOK_FAILURE;
  if (uv_thread_create(&hook_thread, hook_thread_proc, NULL) == 0) {
    uv_thread_t spur;
    uv_thread_create(&spur, spurious_proc, NULL);
    uv_cond_wait(&hook_control_cond, &hook_control_mutex);   // no predicate
    if (uv_mutex_trylock(&hook_running_mutex) == 0) {
      fprintf(stderr, "  [old] premature wakeup -> 'startup problem' branch; joining under both mutexes...\n");
      uv_thread_join(&hook_thread);                           // <-- deadlock
      status = hook_thread_status;
      uv_mutex_unlock(&hook_running_mutex);
    } else {
      status = UIOHOOK_SUCCESS;
    }
  } else {
    status = UIOHOOK_ERROR_THREAD_CREATE;
  }
  uv_mutex_unlock(&hook_control_mutex);
  return status;
}

static int hook_enable_new(void) {
  uv_mutex_lock(&hook_control_mutex);
  hook_state = HOOK_START_PENDING;
  if (uv_thread_create(&hook_thread, hook_thread_proc, NULL) != 0) {
    uv_mutex_unlock(&hook_control_mutex);
    return UIOHOOK_ERROR_THREAD_CREATE;
  }
  uv_thread_t spur;
  uv_thread_create(&spur, spurious_proc, NULL);
  while (hook_state == HOOK_START_PENDING) {
    uv_cond_wait(&hook_control_cond, &hook_control_mutex);
  }
  hook_start_state st = hook_state;
  uv_mutex_unlock(&hook_control_mutex);
  if (st == HOOK_START_RUNNING) return UIOHOOK_SUCCESS;
  uv_thread_join(&hook_thread);
  return hook_thread_status;
}

int main(int argc, char** argv) {
  use_new = (argc > 1 && strcmp(argv[1], "new") == 0);
  uv_mutex_init(&hook_running_mutex);
  uv_mutex_init(&hook_control_mutex);
  uv_cond_init(&hook_control_cond);
  uv_mutex_init(&stop_mutex);
  uv_cond_init(&stop_cond);

  printf("[%s] calling hook_enable()...\n", use_new ? "new" : "old");
  int status = use_new ? hook_enable_new() : hook_enable_old();
  printf("[%s] hook_enable() returned %d (expected 0)\n", use_new ? "new" : "old", status);

  uv_mutex_lock(&stop_mutex);
  stop_requested = 1;
  uv_cond_signal(&stop_cond);
  uv_mutex_unlock(&stop_mutex);
  uv_thread_join(&hook_thread);
  printf("[%s] clean exit\n", use_new ? "new" : "old");
  return status == UIOHOOK_SUCCESS ? 0 : 1;
}

For defect 2, drop the spurious-wakeup thread and make hook_run() return UIOHOOK_FAILURE before dispatch_hook_enabled(), with a short delay in hook_enable() between uv_thread_create() and the wait so the thread's signal is emitted before anyone is waiting on the condvar.

Notes

I did not find an existing issue for this specific startup deadlock. #50 is a launch-time crash (tsfn_to_js_proxy napi_call_function, traced by a commenter to a throwing JS callback), and #23 / #65 are freezes that occur after events start flowing, in input_hook.c. This one is in uiohook_worker.c and happens during hook_enable() itself, before any event is delivered.

The affected code is uiohook-napi's own, not vendored libuiohook, so nothing needs to change upstream in libuiohook. That said, the pattern originates in libuiohook's async demo, so the same reasoning may apply there.

Disclosure: this was root-caused and written with the help of Claude Code — reading the C, building the repro harness, and verifying against a real native rebuild. Every claim above was checked by running it, not inferred.

[alxndalexeev]

Pushed a small revision (force-push, no review had landed yet — the diff is otherwise unchanged).

hook_thread_proc now sets the terminal state unconditionally instead of only when the state was still pending:

uv_mutex_lock(&hook_control_mutex);
hook_state = HOOK_START_FINISHED;   // was: if (hook_state == HOOK_START_PENDING) ...
uv_cond_signal(&hook_control_cond);
uv_mutex_unlock(&hook_control_mutex);

The third enum value is renamed HOOK_START_FAILED → HOOK_START_FINISHED to match what it now means: "hook_run() returned, the thread is joinable" rather than "startup failed".

Why: if the hook enables and then stops again before hook_enable() observes the first signal, the conditional version left the state at HOOK_START_RUNNING, so hook_enable() returned UIOHOOK_SUCCESS for a thread that had already exited and would never be joined — uiohook_worker_stop() skips its join when hook_stop() doesn't return success. Setting it unconditionally makes "the thread has exited" win, which is the more useful answer and keeps the join paired. Vanishingly rare either way, but there's no cost to getting it right.

Re-verified after the change: node-gyp rebuild clean, start() 59 ms / stop() 1 ms with real events captured, 8 start/stop cycles clean, and both repros unchanged — defect 1 old 3/3 hang vs new 5/5 exit 0, defect 2 old 3/3 hang vs new 3/3 correct failure report.