Fix high CPU usage in callback thread due to incorrect timespec #782

2026-03-28T22:37:29-05:00

k1-801 commented

2026-03-28 22:37:29 -05:00

(Migrated from github.com)

@CalcProgrammer1 noticed high CPU usage (50%) on one of the threads when libusb backend is used for hotplug. It was determined that the load was caused by hidapi_thread_cond_timedwait(), which requires an absolute timespec, while the code passes a relative value. This patch copies how the structure is filled from the other place where hidapi_thread_cond_timedwait() is uses, which should fix the issue.

@CalcProgrammer1 noticed high CPU usage (50%) on one of the threads when libusb backend is used for hotplug. It was determined that the load was caused by `hidapi_thread_cond_timedwait()`, which requires an absolute timespec, while the code passes a relative value. This patch copies how the structure is filled from the other place where `hidapi_thread_cond_timedwait()` is uses, which should fix the issue.

Youw (Migrated from github.com) reviewed

2026-03-29 12:30:54 -05:00

libusb/hid.c

					
				@ -1215,16 +1215,16 @@ static void* callback_thread(void* user_data)

				{

I'd set it to 1 second - I've been using 1 second in multiple projects, and seems reasonable
5 second to exit the thread (effectively - to quit the application in some cases) seem a bit too much

I'd set it to 1 second - I've been using 1 second in multiple projects, and seems reasonable 5 second to exit the thread (effectively - to quit the application in some cases) seem a bit too much

copilot-pull-request-reviewer[bot] (Migrated from github.com) reviewed

2026-03-29 13:26:11 -05:00

copilot-pull-request-reviewer[bot] (Migrated from github.com) left a comment

Pull request overview

Fixes high CPU usage in the libusb hotplug callback thread by passing an absolute timeout to hidapi_thread_cond_timedwait() (required by the pthread-backed implementation), instead of a relative timespec.

Changes:

Move timeout computation into the callback loop and build it as now + 5ms via hidapi_thread_gettime() + hidapi_thread_addtime().
Align the hotplug callback thread’s timed-wait behavior with the existing timed-wait usage elsewhere in libusb/hid.c.

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

## Pull request overview Fixes high CPU usage in the libusb hotplug callback thread by passing an **absolute** timeout to `hidapi_thread_cond_timedwait()` (required by the pthread-backed implementation), instead of a relative `timespec`. **Changes:** - Move timeout computation into the callback loop and build it as `now + 5ms` via `hidapi_thread_gettime()` + `hidapi_thread_addtime()`. - Align the hotplug callback thread’s timed-wait behavior with the existing timed-wait usage elsewhere in `libusb/hid.c`. --- 💡 <a href="/libusb/hidapi/new/master?filename=.github/instructions/*.instructions.md" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Add Copilot custom instructions</a> for smarter, more guided reviews. <a href="https://docs.github.com/en/copilot/customizing-copilot/adding-repository-custom-instructions-for-github-copilot" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Learn how to get started</a>.

k1-801 (Migrated from github.com) reviewed

2026-03-30 03:36:16 -05:00

libusb/hid.c

					
				@ -1215,16 +1215,16 @@ static void* callback_thread(void* user_data)

				{

According to the call signature, the number is in milliseconds, not whole seconds.

When it comes to milliseconds, it doesn't really matter what number is set there, as execution at single milliseconds scale is likely limited by OS scheduler, not the actual number set here, so long as it's long enough to actually go idle.

A whole second here would be too long (at least to my taste), and checking every single millisecond seems a bit excessive. It can be changed later on to 20 or 40 or even 100 milliseconds if needed (i.e. if this thread makes any noticeable CPU load).

According to the call signature, the number is in milliseconds, not whole seconds. <img width="863" height="312" alt="image" src="https://github.com/user-attachments/assets/3682a469-5737-4b7b-95d9-c16a76cec15b" /> When it comes to milliseconds, it doesn't really matter what number is set there, as execution at single milliseconds scale is likely limited by OS scheduler, not the actual number set here, so long as it's long enough to actually go idle. A whole second here would be too long (at least to my taste), and checking every single millisecond seems a bit excessive. It can be changed later on to 20 or 40 or even 100 milliseconds if needed (i.e. if this thread makes any noticeable CPU load).

k1-801 (Migrated from github.com) reviewed

2026-03-30 03:49:38 -05:00

libusb/hid.c

					
				@ -1215,16 +1215,16 @@ static void* callback_thread(void* user_data)

				{

So far, the patch seems to work:

("wrapped HID devices" in this context means HID devices that use libusb backend specifically)

So far, the patch seems to work: <img width="979" height="421" alt="image" src="https://github.com/user-attachments/assets/01b1d5a5-c6f7-4754-8c07-443a592959c1" /> ("wrapped HID devices" in this context means HID devices that use libusb backend specifically)

Youw (Migrated from github.com) reviewed

2026-03-30 04:21:54 -05:00

libusb/hid.c

					
				@ -1215,16 +1215,16 @@ static void* callback_thread(void* user_data)

				{

While modern processors are fast-enough practically not to notice wait/idle/check every 5ms, my experience tells it is a bit too often, I'd make it 100ms then.

k1-801 (Migrated from github.com) reviewed

2026-03-30 05:21:17 -05:00

libusb/hid.c

					
				@ -1215,16 +1215,16 @@ static void* callback_thread(void* user_data)

				{

Increased the timeout to 100

Youw commented

2026-03-30 05:43:51 -05:00

(Migrated from github.com)

Meanwhile I'm looking at a fundamentally different aproach to fix this issue - there should be no timeout at all: #783

k1-801 commented

2026-03-30 05:54:34 -05:00

(Migrated from github.com)

The approach suggested there would also work; however, the proposed change simultaneously breaks the cache of connected devices, which would lead to inability to report disconnect events.

I don't remember exactly why I started with a timed cond_wait, it was either a precaution against infinite wait if, or the exact reason could have been resolved.

The approach suggested there would also work; however, the proposed change simultaneously breaks the cache of connected devices, which would lead to inability to report disconnect events. I don't remember exactly why I started with a timed `cond_wait`, it was either a precaution against infinite wait if, or the exact reason could have been resolved.

Youw commented

2026-03-30 12:18:02 -05:00

(Migrated from github.com)

@k1-801
Check the PR again - there are also a few multithreading bugs fixed.

a timed cond_wait, it was either a precaution against infinite wait if

There is a high change that because of this:
https://github.com/libusb/hidapi/pull/783/changes#diff-516ed9d234e81614e6fa65cfc81a50b8b119e8a0769b2e66a1c89166bac437f7R1286-R1288

There is exacty one reason why a timed wait should be used in cases like that - when it is impossible to single the even in a clean way in any other was. In this case - it is a 100% possible and should be done exactly like that.

@k1-801 Check the PR again - there are also a few multithreading bugs fixed. > a timed cond_wait, it was either a precaution against infinite wait if There is a high change that because of this: https://github.com/libusb/hidapi/pull/783/changes#diff-516ed9d234e81614e6fa65cfc81a50b8b119e8a0769b2e66a1c89166bac437f7R1286-R1288 There is exacty one reason why a timed wait should be used in cases like that - when it is impossible to single the even in a clean way in any other was. In this case - it is a 100% possible and should be done exactly like that.

mcuee commented

2026-04-24 07:27:01 -05:00

(Migrated from github.com)

#783 has been merged. Do we still need this one?

Youw commented

2026-04-24 09:18:12 -05:00

(Migrated from github.com)

Interesting, it should have been closed on its own

CalcProgrammer1 commented

2026-04-24 09:30:02 -05:00

(Migrated from github.com)

Is there a new "finished" hotplug branch available? If so let me know which branch and I will update my fork and test it in OpenRGB next this weekend across Windows, Linux, and MacOS.

mcuee commented

2026-04-24 09:33:07 -05:00

(Migrated from github.com)

Is there a new "finished" hotplug branch available? If so let me know which branch and I will update my fork and test it in OpenRGB next this weekend across Windows, Linux, and MacOS.

https://github.com/libusb/hidapi/tree/connection-callback

Not yet finished...

> Is there a new "finished" hotplug branch available? If so let me know which branch and I will update my fork and test it in OpenRGB next this weekend across Windows, Linux, and MacOS. https://github.com/libusb/hidapi/tree/connection-callback Not yet finished...

CalcProgrammer1 commented

2026-04-24 09:36:03 -05:00

(Migrated from github.com)

That's fine, I will test this latest branch. I meant finished in terms of this issue mainly.

👍 1

CalcProgrammer1 commented

2026-04-24 11:24:45 -05:00

(Migrated from github.com)

With the updated connection-callback branch I am seeing about 0.7% CPU usage in htop from the openrgb background service on Linux, which is in line with what I would expect. I don't have the devices on me to test the wrapped/libusb backend right now but the hidraw backend successfully detects hotplug and removal. Both backends have their hotplug listening threads running as part of that 0.7%.