Hi, On 07/10/2012 02:40 PM, Peter Stuge wrote: > Hans de Goede wrote: >> I had looking further into this / writing a patch on my todo list, >> looks like you beat me to it, thanks for working on this! >> >> You're patch looks good, one possible optimization would be to >> give arm_timerfd_for_next_timeout() an extra "int stop_timer" >> argument and only call disarm_timerfd() when that argument is true. > > The proposed patch is a bit more complicated than neccessary, and > arm_timerfd_for_next_timeout() uses the return value to indicate > if a timer is now running or not. Thanks to debugging by Vincent > the bug was fixed in libusb yesterday. > > I'm attaching a patch that applies on top of libusbx.git, and for > convenience it is also available via
Thanks! And good to see confirmed that there actually is an issue there which needs fixing. With that said I think Pete's fix is better though, doing the disarm without the flying-transfers lock held is racy, ie: Thread 1: -entering usbi_handle_transfer_completion() -calls arm_timerfd_for_next_timeout(), which returns 0 -unlock flying transfer-lock Switch to thread 2 -entering libusb_submit_transfer() -calls add_to_flying_list(), which returns 1 -sets up the timerfd for the just submitted transfer, as add_to_flying_list() returned 1 Switch back to thread 1: -check arm_timerfd_for_next_timeout() return val -retval == 0, so disarm timer_fd And now the timer_fd has been disabled even though it should be set to fire for the timeout of the transfer submitted by thread 2. Speaking more in general, the timerfd is a single resource shared by all transfers, so it should be protected from concurent accesses. So I not only think Pete's patch is better, IMHO the setting of the timerfd from libusb_submit_transfer should also be moved to add_to_flying_list() and be done under the protected of the flying-transfers lock. Regards, Hans ------------------------------------------------------------------------------ Live Security Virtual Conference Exclusive live event will cover all the ways today's security and threat landscape has changed and how IT managers can respond. Discussions will include endpoint security, mobile security and the latest in malware threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/ _______________________________________________ libusbx-devel mailing list libusbx-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/libusbx-devel
