Libusb updates #2
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* Issue reported by Thejus
* Also update Windows version report for Windows 10
* Var found was not reset to false before list_for_each_entry() * ctx->open_devs_lock was not released on error. * Issues reported by Yongjian Xu
This page lists the all the public functions, structures and enums provided by libusb. The HTML page is generated by Doxygen.
The structure libusb_hotplug_callback was declared (but not defined) and never used in the public API.
libusb/hotplug.c:46: warning: multiple use of section label 'intro' while adding section, (first occurrence: libusb/core.c, line 79) Use label "hotplug_intro" instead of "intro"
libusb/io.c:46: warning: multiple use of section label 'intro' while adding section, (first occurrence: libusb/core.c, line 79) Use label "io_intro" instead of "intro"
Warning: Tag `XML_SCHEMA' at line 942 of file `doxygen.cfg' has become obsolete.
To avoid this warning please remove this line from your configuration file or upgrade it using "doxygen -u"
Warning: Tag `XML_DTD' at line 948 of file `doxygen.cfg' has become obsolete.
To avoid this warning please remove this line from your configuration file or upgrade it using "doxygen -u"
I upgrade the doxygen.cfg.in using: "doxygen -u doxygen.cfg.in"
The configuration file changed from Doxyfile 1.5.3 to Doxyfile 1.8.8
It is unnecessary to take the events lock when a thread needs to interrupt an event handler to receive a change list of poll fds. It is sufficient to simply write to the control pipe and let the event handler be notified of this event when it polls. Taking the events lock inside this function leads to opportunity for deadlock in certain situations, such as a client program running on an OS that uses fd notification on each individual transfer. If the client program were to protect a list of transfers with a single lock, it could deadlock if that lock were taken in two separate threads, one which is attempting to submit a new transfer and one which is executing inside the transfer completion callback. Thanks to Dmitry Fleytman and Pavel Gurvich for reporting this. Signed-off-by: Chris Dickens <[email protected]>
This counter is now solely used for closing a device, so rename the variable appropriately. Signed-off-by: Chris Dickens <[email protected]>
This lock will be used in subsequent changes that will consolidate all different event sources (i.e. device close, fd notification, hotplug message) into a single event. Signed-off-by: Chris Dickens <[email protected]>
This change removes the device_close_lock and uses the shared event data lock to protect the value of the device_close counter. Signed-off-by: Chris Dickens <[email protected]>
Signed-off-by: Chris Dickens <[email protected]>
Signed-off-by: Chris Dickens <[email protected]>
This flag will be useful in a subsequent commit that further consolidates event handling. Signed-off-by: Chris Dickens <[email protected]>
To further consolidate libusb internal events, this change removes the hotplug pipe. Hotplug messages are now kept in a list within the context, and the event pipe is signalled when a new hotplug message is added. When handling events, the hotplug messages will be processed from the list instead of from a separate pipe. This change is greatly beneficial for the Windows/WinCE backends which do not allow pipes to be used in the WaitFor* functions. Signed-off-by: Chris Dickens <[email protected]>
This change will ensure that the event pipe is only signalled at most one time during the course of any incoming events. New events that occur while the event pipe is in the signalled state will not cause the event pipe to be signalled again. This change will provide support for the use of native events on the Windows/WinCE backends, as these events are binary and do not "count" the number of times they are signalled. Signed-off-by: Chris Dickens <[email protected]>
This change consists of two parts that must be taken together. Part 1 moves the pollfd list under the protection of event_data_lock and eliminates the pollfd_lock. Since modifications to the pollfd list are considered an event, it makes sense to merge this. Another benefit of doing so is an enhancement to event handling. The event handler can get the updated pollfd list upon entry into handle_events() and can clear the event pipe if no other pending events exist, which saves a needless iteration. Part 2 makes notification of pollfd list changes part of adding or removing a pollfd from the list. Previously this was done in two distinct steps, however nothing prevented a new pollfd from being used by an event handler before an explicit notification was sent out. This change eliminates the need for USBI_TRANSFER_UPDATED_FDS and reverts 9a9ef3e. Signed-off-by: Chris Dickens <[email protected]>
Control requests on composite devices for Windows is tricky, because not all APIs are able to correctly process all types of requests. To improve robustness, this change checks whether the control request recipient is an interface or endpoint. If it is either, the control request will be attempted against that interface's API first. Additionally, when cycling through the interfaces to find a working API, it will no longer fail if that API does not support the specific type of control request because there is a chance that another API will succeed. Signed-off-by: Chris Dickens <[email protected]>
Hans pointed out that usbi_fd_notification() is only used by the functions that add/remove pollfds. They already hold the required lock, so to make it less expensive we will assume the lock is already held. The usbi_fd_notification() function has also been moved from core.c into io.c and made static, since it now only has one use case. Signed-off-by: Chris Dickens <[email protected]>
…n OSX versions older than 10.9. Closes libusb#48 Signed-off-by: Nathan Hjelm <[email protected]>
Signed-off-by: Chris Dickens <[email protected]>
* libusbK (as of v3.0.7.0) will fail after 32 opens because resources from claimed interfaces are not freed by simply closing just the device handle * Closes libusb#16 Signed-off-by: Chris Dickens <[email protected]>
In the docs, we claim to report LIBUSB_ERROR_BUSY if submitting a transfer that has already been submitted. Linux was the only backend that actually checked and reported this, but unfortunately the code in libusb_submit_transfer() resulted in a segfault. This is because submission failure would delete the (active) transfer from the flying_transfers list, and when the transfer finally completes it would be deleted from the flying_transfers list a second time. Instead of modifying each backend to check for a busy transfer, this patch adds a flag to indicate when a transfer is in-flight. The core library will check this flag and return LIBUSB_ERROR_BUSY immediately. This patch also modifies libusb_cancel_transfer() to check that a transfer is in-flight before cancelling and to check that a transfer has not already been cancelled, returning LIBUSB_ERROR_NOT_FOUND in both cases. Signed-off-by: Chris Dickens <[email protected]>
Device leak occurred if either of the following occured: 1) Detach followed by attach 2) Two consecutive attach Signed-off-by: Chris Dickens <[email protected]>
* Closes libusb#35 Signed-off-by: Chris Dickens <[email protected]>
Currently all non-Linux POSIX backends emulate the Linux design of having individual pollable file descriptors for each device. This patch introduces a new internal API that allows backends to handle transfer completion notification in a different way. A new function, usbi_signal_transfer_completion(), is added. Each backend can call this function when it detects a transfer has completed. This function will place the transfer on a new completed_transfers list and will signal the context's event. Backends that use this new API will no longer provide a handle_events() function but instead provide a handle_transfer_completion() function. When a thread is doing event handling, it will call this backend function for all transfers on the completed_transfers list. With this change, backends that do not really have natively pollable file descriptors can completely move away from this design and avoid poll() altogether. Signed-off-by: Chris Dickens <[email protected]>
This commit is based on Nathan's branch https://github.com/hjelmn/libusb-darwin/blob/master/examples/testlibusb1.c Closes libusb#178 Signed-off-by: Nathan Hjelm <[email protected]>
OS X does not support unnamed semaphores so the example has been updated to use a named semaphore instead. Signed-off-by: Nathan Hjelm <[email protected]>
Signed-off-by: Nathan Hjelm <[email protected]>
Signed-off-by: Nathan Hjelm <[email protected]>
There is a race between handle_timeout() the completion functions. When one thread is in handle_timeout() and another thread wakes up from a poll(), there exists a window where the transfer has been cancelled, but its USBI_TRANSFER_TIMED_OUT flag is not set yet. Therefore, usbi_handle_transfer_completion() is sometimes called with LIBUSB_TRANSFER_CANCELLED instead of the expected LIBUSB_TRANSFER_TIMED_OUT. This change adds transfer and flag locks to the handle_timeout() function. Closes libusb#197 Signed-off-by: Nathan Hjelm <[email protected]>
Signed-off-by: Nathan Hjelm <[email protected]>
Signed-off-by: Nathan Hjelm <[email protected]>
Closes libusb#177 Signed-off-by: Nathan Hjelm <[email protected]>
Signed-off-by: Nathan Hjelm <[email protected]>
Closes libusb#198 Signed-off-by: Nathan Hjelm <[email protected]>
Signed-off-by: Nathan Hjelm <[email protected]>
This reverts commit bd8d5b5. Chris Dickens and me have been working on a patch-set refactoring the transfer flag handling which fixes this differently. Revert this commit so that the refactoring changes can be merged cleanly. Signed-off-by: Hans de Goede <[email protected]>
The event handling lock was previously required to be of the recursive type because the libusb_close() path requires the lock and may be called by a thread that is handling events (e.g. from within a transfer or hotplug callback). With commit 960a6e7, it is possible to determine whether the current function is being called from an event handling context, thus the recursive lock type is no longer necessary. References: * http://libusb.org/ticket/82 * 7428258 * c775c2f Signed-off-by: Chris Dickens <[email protected]> Reviewed-by: Hans de Goede <[email protected]> [[email protected]: rebase on top of current master] Signed-off-by: Hans de Goede <[email protected]>
(itransfer->flags & USBI_TRANSFER_TIMED_OUT) is already checked by usbi_handle_transfer_cancellation(), which wince_transfer_callback() will call when status == LIBUSB_TRANSFER_CANCELLED. Leave this up to the core, so that future changes to timeout handling do no break wince. Signed-off-by: Hans de Goede <[email protected]> --- Note: untested
(itransfer->flags & USBI_TRANSFER_TIMED_OUT) is already checked by usbi_handle_transfer_cancellation(), make windows_transfer_callback() call usbi_handle_transfer_cancellation() when status == LIBUSB_TRANSFER_CANCELLED like all other os backends do, and leave USBI_TRANSFER_TIMED_OUT handling up to the core, so that future changes to timeout handling do no break winnt. Signed-off-by: Hans de Goede <[email protected]> --- Note: untested
Commit a886bb0 sped up the library a bit by removing the serialization of transfer submission with respect to the flying_transfers list, but it introduced two separate issues. 1) A deadlock scenario is possible given the following sequence: - Thread A submits transfer with very short timeout (say 1ms) -> takes transfer->lock -> adds transfer to flying_transfers list and arms timerfd -> actually calls backend to submit transfer, but it fails <context switch> - Thread B is doing event handling and sees the timerfd trigger -> takes ctx->flying_transfers_lock -> finds the transfer above on the list -> calls libusb_cancel_transfer() for this transfer --> takes transfer->lock <context switch> - Thread A sees the transfer failed to submit -> removes transfer from flying_transfers list --> takes ctx->flying_transfers_lock (still holding transfer->lock) ** DEADLOCK ** 2) The transfer state flags (e.g. submitting, in-flight) were protected by transfer->flags_lock, but the timeout-related flags were OR'ed in during timeout handling operations outside of the lock. This leads to the possibility that transfer state might get overwritten. This change corrects these issues and simplifies the transfer submission code a bit by separating the state and timeout flags into their own flag variables. The state flags are protected by the transfer lock. The timeout flags are protected by the flying_transfers_lock. The transfer submission code sheds some weight because it no longer needs to worry about the timing of events that modify the transfer state flags. These flags are always viewed and modified under the protection of the transfer lock. Since libusb_submit_transfer() holds the transfer lock for the entire duration of the operation, the other code paths that would possibly touch the transfer (e.g. usbi_handle_disconnect() and usbi_handle_transfer_completion()) have to wait for transfer submission to fully complete. This eliminates any possible race conditions. Signed-off-by: Chris Dickens <[email protected]> [[email protected]: Reworked libusb_submit_transfer changes so that in case both flying_transfer_lock and itransfer->lock are taken flying_transfers_lock is always taken first] [[email protected]: Removed some unrelated changes (will be submitted as separate patches)] Signed-off-by: Hans de Goede <[email protected]>
Signed-off-by: Hans de Goede <[email protected]>
Signed-off-by: Hans de Goede <[email protected]>
Put the lock / unlock calls around the part of the code which actually checks the flags which the lock protect. Signed-off-by: Hans de Goede <[email protected]>
There is a race between handle_timeout() and the completion functions. When one thread is in handle_timeout() and another thread wakes up from a poll(), there exists a window where the transfer has been cancelled, but USBI_TRANSFER_TIMED_OUT is not yet set in timeout_flags. Therefore, usbi_handle_transfer_completion() is sometimes called with LIBUSB_TRANSFER_CANCELLED instead of the expected LIBUSB_TRANSFER_TIMED_OUT. timeout_flags is protected by the flying_transfers_lock, this commit makes usbi_handle_transfer_cancellation() take that lock before checking for USBI_TRANSFER_TIMED_OUT in timeout_flags, fixing this. Reported-by: Joost Muller <[email protected]> Signed-off-by: Hans de Goede <[email protected]>
This cleans-up libusb_submit_transfer a bit by avoiding an error exit path with unlock calls. Signed-off-by: Hans de Goede <[email protected]>
…mpleted The linux kernel will set its internal device state to USB_STATE_NOTATTACHED as soon as it detects the disconnect, and then start a worker thread to deal with the actual disconnection, kill outstanding urbs, etc. The usbfs poll implementation will return POLL_ERR as soon as ps->dev->state == USB_STATE_NOTATTACHED. The kernel will not wakeup the poll until it is done with processing the disconnection. But if we happen to call poll() between the state change and the disconnection being fully processed, we may not be able to reap all outstanding transfers, even on kernels with the USBFS_CAP_REAP_AFTER_DISCONNECT capability. This commit deals with this by trying to reap as many transfers as possible on disconnect on USBFS_CAP_REAP_AFTER_DISCONNECT capable kernels and then calling usbi_handle_disconnect(handle) to deal with any remaining ones. On USBFS_CAP_REAP_AFTER_DISCONNECT capable kernels this will be a no-op unless we hit the race. Signed-off-by: Hans de Goede <[email protected]>
…ansfer Signed-off-by: Hans de Goede <[email protected]>
Signed-off-by: Nathan Hjelm <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.