Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use separate libusb contexts to avoid thread safety issues #8218

Merged
merged 2 commits into from Jul 11, 2019

Conversation

2 participants
@leoetlino
Copy link
Member

commented Jun 26, 2019

Unfortunately, it appears that using libusb's synchronous transfer API
from several threads causes nasty race conditions in event handling and
can lead to deadlocks, despite the fact that libusb's synchronous API
is documented to be perfectly fine to use from several threads (only
the manual polling functionality is supposed to require special
precautions).

Since usbdk was the only real reason for using a single libusb context
and since usbdk (currently) has so many issues with Dolphin, I think
dropping support for it in order to fix other backends is acceptable.

And while we're at it, fix an invalid CoreTiming::FromThread value.

leoetlino added some commits Jun 16, 2019

BTReal: Use FromThread::ANY for USB transfer replies
libusb transfer callbacks might be called immediately during transfer
submission in some cases. (libusb doesn't even specify what thread
the callback is invoked on.) In other words, it is possible to reach
the transfer callback from the CPU thread, and not just from the
USB event handling thread.

So CoreTiming::FromThread::NON_CPU is incorrect and should instead
be ANY.
Use separate libusb contexts to avoid thread safety issues
Unfortunately, it appears that using libusb's synchronous transfer API
from several threads causes nasty race conditions in event handling and
can lead to deadlocks, despite the fact that libusb's synchronous API
is documented to be perfectly fine to use from several threads (only
the manual polling functionality is supposed to require special
precautions).

Since usbdk was the only real reason for using a single libusb context
and since usbdk (currently) has so many issues with Dolphin, I think
dropping support for it in order to fix other backends is acceptable.
@stenzek

stenzek approved these changes Jul 7, 2019

Copy link
Contributor

left a comment

Not familiar with how libusb is integrated, but the code changes LGTM

@leoetlino leoetlino merged commit 2f5e6cf into dolphin-emu:master Jul 11, 2019

10 checks passed

default Very basic checks passed, handed off to Buildbot.
Details
lint Build succeeded on builder lint
Details
pr-android Build succeeded on builder pr-android
Details
pr-deb-dbg-x64 Build succeeded on builder pr-deb-dbg-x64
Details
pr-deb-x64 Build succeeded on builder pr-deb-x64
Details
pr-freebsd-x64 Build succeeded on builder pr-freebsd-x64
Details
pr-osx-x64 Build succeeded on builder pr-osx-x64
Details
pr-ubu-x64 Build succeeded on builder pr-ubu-x64
Details
pr-win-dbg-x64 Build succeeded on builder pr-win-dbg-x64
Details
pr-win-x64 Build succeeded on builder pr-win-x64
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.