search

Microchip-MPLAB-Harmony / usb

Harmony 3 USB library
https://onlinedocs.microchip.com/v2/keyword-lookup?keyword=MH3_usb&redirect=true
Other
12 stars 7 forks source link

Deadlock in `USB_DEVICE_Detach()` #19

Open js-nano opened 2 years ago

js-nano commented 2 years ago

There's a deadlock when calling USB_DEVICE_Detach()

Cause of the deadlock

If USB_DEVICE_Detach() is called from a non-interrupt context, a mutex is locked:

https://github.com/Microchip-MPLAB-Harmony/usb/blob/204d5f2d29d7ced2e9bd42cb9c3a4e4546e8fdfd/driver/usbdp/src/dynamic/drv_usbdp_device.c#L842

USB_DEVICE_Detach() then calls DRV_USBDP_EndpointDisable()...

https://github.com/Microchip-MPLAB-Harmony/usb/blob/204d5f2d29d7ced2e9bd42cb9c3a4e4546e8fdfd/driver/usbdp/src/dynamic/drv_usbdp_device.c#L859

... which then proceeds to try to lock the same mutex, with a timeout of OSAL_WAIT_FOREVER:

https://github.com/Microchip-MPLAB-Harmony/usb/blob/204d5f2d29d7ced2e9bd42cb9c3a4e4546e8fdfd/driver/usbdp/src/dynamic/drv_usbdp_device.c#L1232

This sequence is guaranteed to cause a deadlock (assuming USB_DEVICE_Detach() is not called from an interrupt context).

Steps leading to the deadlock

I encountered this problem when trying to use both FreeRTOS and the default USB stack on a SAM G55 XPlained Pro board.

The docs say to call USB_DEVICE_Detach() in response to the USB_DEVICE_EVENT_POWER_REMOVED (1).

But the VBUS monitoring occurrs in DRV_USBDP_Tasks(), which is called in a task context (not an interrupt context) when using FreeRTOS. This means that when the USB_DEVICE_EVENT_POWER_REMOVED event is passed into the task event handler, the task is not running in an interrupt context

Why does the demo project work?

Calling USB_DEVICE_Detach() in the G55 cdc_com_port_single demo project works, because the OSAL implementation used is osal_implementation_basic.h. In this implementation, the mutex locking doesn't take the timeout into account, so the deadlock doesn't occur: https://github.com/Microchip-MPLAB-Harmony/usb_apps_device/blob/e8089912b07a7c4eaec5d5ee81c6929badfc9a9f/apps/cdc_com_port_single/firmware/src/config/sam_g55_xpro/osal/osal_impl_basic.h#L274-L282

I've not got the hardware to test one of the FreeRTOS demo projects, so I can't say if they work

Thanks, -Jon

js-nano commented 2 years ago

Modifying osal_freertos.h so that all the OSAL_Mutex functions use a recursive mutex solves the problem, indicating that the problem is indeed caused by the mutex locking.

otonoton commented 3 months ago

Are you using polled usb mode instead of the default interrupt mode? I believe that is not recommended, but I agree this shouldn't happen either way. FWIW my event handler by default runs in an interrupt context and Detach() works fine.