fix: only start one hotplug watcher, so connect fires once per plug

[TKDubsta]

fix: only start one hotplug watcher, so connect fires once per plug

I ran into this moving an Electron app over from node-usb 2.x: a single plug-in was
triggering our USB alert rules twice. connect fires twice for one physical plug on
3.0.1.

start_watching checks whether a watcher is already running and then stores the new
one, but lets go of the lock in between. addEventListener kicks off addAttach and
addDetach back to back without awaiting either, so they run at the same time, both
see an empty slot, and both spawn a watcher. Two watchers, so every hotplug event gets
delivered twice.

disconnect only looks fine by accident — the TS side checks knownDevices.has(handle)
and deletes the handle as it fires, so the duplicate gets swallowed there. connect has
no equivalent check, which is why you get two connects but one disconnect.

Looks like it arrived with the watch_task refactor in #18. 3.0.0 did this a different
way and isn't affected.

Reproducing it

You don't need a device for this — the race is on registration, not on the plug. Drop an
eprintln!("spawned") immediately before the tokio::spawn in start_watching, then:

const { webusb } = require('./dist');
// the first listener of either type kicks off both addAttach and addDetach
webusb.addEventListener('connect', () => {});
setTimeout(() => process.exit(0), 750);

On main that prints twice, every time — 50 out of 50 runs here, always from two
different threads. With the patch it prints once, 50 out of 50.

On actual hardware, replugging a Brother PT-P950NW on 3.0.1 gave 1 disconnect and 2
connects (same tick, same serial), against 1 and 1 on node-usb 2.18.0.

The fix

Hold one lock across both the check and the store, which means putting watch_task
behind an Arc<Mutex<..>> the way callbacks already is.

Ive also taken the four napi methods from &mut self to &self. napi can poll them
concurrently, so &mut self was handing out aliasing &mut — the duplicate watcher is
really just the visible symptom of that. They don't need to be unsafe any more either,
which clears four clippy warnings. Neither start_watching nor stop_watching has an
await in it now, so both drop async.

I checked it doesn't over-correct: remove both listeners and re-register, and you get
exactly one fresh watcher, so teardown still works. Generated index.d.ts is unchanged,
and fmt, build and tsc all pass.

Things you might want done differently

This could be fixed in the TS instead, by chaining the two native calls so they can't
overlap — probably a two-line change. I went the Rust route because the gap is open to
any concurrent caller rather than just that one call site, and the &mut self seemed
worth getting rid of regardless. Happy to redo it the other way if you'd rather.

The lock is now held across nusb::watch_devices(). It's only contended while listeners
are being set up, so I don't think it matters in practice, but if you'd sooner not hold
it there, moving the stream creation inside the spawned task would narrow it to just the
store.

Only built and run on macOS arm64.

[Copilot]

Pull request overview

This PR addresses a concurrency race in the Rust-side hotplug watcher management so that a single physical USB plug-in results in exactly one connect event, aligning behavior with node-usb 2.x expectations and preventing duplicate event delivery in consumers (e.g., Electron apps).

Changes:

• Makes watch_task shared/mutable via Arc<Mutex<Option<JoinHandle<()>>>> and holds a single lock across the “is watcher running?” check and storing the spawned task to prevent double-spawn.
• Removes unnecessary unsafe, switches N-API methods from &mut self to &self, and makes start_watching / stop_watching synchronous since they no longer await.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.