未验证 提交 ccf0d877 编写于 作者: R RefactorFactory 提交者: GitHub

Fix race condition with USBHID semaphore (#7205)

The HID semaphore allows USBHID::SendReport() to wait for the completion of
report sending.

With a zero timeout xSemaphoreTake() after calling tud_hid_n_report(),
occasionally, the following would happening:

1. USBHID::SendReport() would send a report by calling tud_hid_n_report().
2. The send would complete and (presumably on another thread)
   tud_hid_report_complete_cb() would be called and it would xSemaphoreGive()
   the semaphore.
3. In USBHID::SendReport(), the zero timeout xSemaphoreTake(sem, 0) would
   succeed, taking the semaphore.
4. On the next line, xSemaphoreTake(sem, timeout_ms ...) would timeout
   because the semaphore was already taken by the previous line of code.

The result would be waiting timeout_ms for no reason.

The purpose of the zero timeout xSemaphoreTake() is to clear the semaphore in
case a previous SendReport() timed out waiting for the semaphore. In that case,
tud_hid_report_complete_cb() may be called after the timeout, giving the
semaphore. Then the next SendReport() would start with the semaphore given,
which isn't desired if we want to call xSemaphoreTake(sem, timeout_ms ...) on
it.

There have also been other cases where tud_hid_report_complete_cb() is called
an extra time, causing the same situation.

The fix is to move the zero timeout xSemaphoreTake() before the call to
tud_hid_n_report(). This eliminates the race between the zero timeout
xSemaphoreTake() and tud_hid_report_complete_cb() in the common case when no
timeout occurs.

There is still a possible race condition between the zero timeout
xSemaphoreTake() and tud_hid_report_complete_cb() in the case of a timeout,
but that should be rarer.
上级 b473fc69
......@@ -345,11 +345,16 @@ bool USBHID::SendReport(uint8_t id, const void* data, size_t len, uint32_t timeo
if(!res){
log_e("not ready");
} else {
// The semaphore may be given if the last SendReport() timed out waiting for the report to
// be sent. Or, tud_hid_report_complete_cb() may be called an extra time, causing the
// semaphore to be given. In these cases, take the semaphore to clear its state so that
// we can wait for it to be given after calling tud_hid_n_report().
xSemaphoreTake(tinyusb_hid_device_input_sem, 0);
res = tud_hid_n_report(0, id, data, len);
if(!res){
log_e("report %u failed", id);
} else {
xSemaphoreTake(tinyusb_hid_device_input_sem, 0);
if(xSemaphoreTake(tinyusb_hid_device_input_sem, timeout_ms / portTICK_PERIOD_MS) != pdTRUE){
log_e("report %u wait failed", id);
res = false;
......
Markdown is supported
0% .
You are about to add 0 people to the discussion. Proceed with caution.
先完成此消息的编辑!
想要评论请 注册