Race condition in msc_file_explorer's wait_for_disk_io causes hang

[DRNadler]

Operating System

Windows 10

Board

imxRT1024

Firmware

This appears to be a generic problem for FreeRTOS (or any RTOS) applications...
Custom FreeRTOS application, using tinyUSB host mode with a USB stick attached.
Per tinyUSB examples, my FreeRTOS application has a task dedicated to processing USB events as follows:

void usb_host_task(void*) {
  tuh_init(BOARD_TUH_RHPORT);  // Initialize device stack on configured roothub port.
  while (1) {
    tuh_task();  // blocks until at least one event is processed, then returns
  }
}

What happened ?

0. USB stick mounted and file system mounted within usb_host_task.

1. Application initiates file IO in a different task (in this case a directory read from FatFS, but could be anything).

2. tinyUSB starts IO and calls msc_file_explorer's wait_for_disk_io routine:

static void wait_for_disk_io(BYTE pdrv)
{
  while(_disk_busy[pdrv])
  {
    tuh_task(); // DRN: Causes race condition: now two tasks awaiting USB events in two instances of tuh_task()
  }
}

3. IO completes and sends an event into the tinyUSB event queue
4. USB task shown in above Firmware section processes the USB event
5. wait_for_disk_io in application task never returns and application hangs

How can this work consistently? The event can be routed by the OS to any thread waiting on the tinyUSB queue. If it happens to get routed to the application thread trying to do file IO, things work fine. But if the event is routed to the USB task, the application hangs.

What am I missing? Isn't this a logic bug?

How to reproduce ?

This is a logic issue... And will be erratic depending on which task receives the event...

Further Discussion

In the example, tuh_task() is actually called recursively: When a USB drive is mounted, the inquiry_complete_cb callback is invoked from inside the event loop. When the callback does further IO, the event queue must be serviced, so tuh_task() is called here recursively. That's OK when everything is in the USB task (as while mounting a USB disk), but when the application tries to do IO in a different thread this yields the race condition above... Here's an example recursive stack trace:

disk io_complete at tinyUSB_to_FatFS.c:228
msch_xfer_cb() at msc_host.c:367
tuh_task_ext() at usbh.c:476
tuh_task() at usbh.h:125
wait_for_disk_io() at tinyUSB_to_FatFS.c:218
disk_read() at tinyUSB_to_FatFS.c:259
move_window() at ff.c:1,090
check_fs() at ff.c:3,301
find_volume() at ff.c:3,340
mount_volume() at ff.c:3,441
f_mount() at ff.c:3,706
inquiry_complete_cb() at tinyUSB_to_FatFS.c:134
msch_xfer_cb() at msc_host.c:367
tuh_task_ext() at usbh.c:476
tuh_task() at usbh.h:125
usb_host task() at Sensorbox_USB.cpp: 70

Possible Fix

Here's how I've fixed this problem for my application but its a bit fragile. Suggestions on how to improve this would be welcome!

static void wait_for_disk_io(BYTE pdrv)
{
  extern TaskHandle_t USB_task_handle;
  bool inUSBtask = (USB_task_handle == xTaskGetCurrentTaskHandle());
  while(_disk_busy[pdrv])
  {
	  // Here we have a bit of a problem. "Disk Busy" is cleared by processing one or more
	  // USB events in the tuh_task event processing loop. But, if we're in the USB task,
	  // we may be within the tuh_task event loop! For example: inquiry_complete_cb is called
	  // in the event loop, and it in turn does IO, which lands us here. The example
	  // shows a (possibly recursive) call to tuh_task(). If we're in the USB task apparently
	  // this recursion is OK, but in a different task it causes a race condition because now
	  // two threads are awaiting USB events, and if the other thread processes the event
	  // this thread will hang.
	  if(inUSBtask)
		  tuh_task(); // this is a recursive invocation!
	  else
		  taskYIELD(); // USB task must be higher priority to process pending events!
  }
}

I have checked existing issues, discussion and documentation

• I confirm I have checked existing issues, discussion and documentation.