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

Fix shutdown race with hotplug_libusb rescan pipe #173

Conversation

emaxx-google
Copy link
Contributor

Fix the possibility that the hotplug_libusb rescan thread remains read()'ing from the rescan pipe throughout the daemon shutdown process.

This is possible because the |rescan_pipe| is created with some delay after the hotplug mechanism is initialized:
HPEstablishUSBNotifications() first notifies about the initialization via writing to |pipefd|, and only then creates |rescan_pipe|. So if HPStopHotPluggables() is called in between, it'll end up doing nothing, letting the hotplug thread sit infinitely long on the read() call meanwhile the main daemon thread is shutting down.

The proposed fix is to create |rescan_pipe| in advance, so that the shutdown logic is guaranteed to be able to write a byte to it, notifying the background thread about the shutdown regardless of when it happens.

@emaxx-google emaxx-google marked this pull request as ready for review April 12, 2024 20:27
@LudovicRousseau
Copy link
Owner

Please rebase

Fix the possibility that the hotplug_libusb rescan thread remains
read()'ing from the rescan pipe throughout the daemon shutdown process.

This is possible because the |rescan_pipe| is created with some delay
after the hotplug mechanism is initialized:
HPEstablishUSBNotifications() first notifies about the initialization
via writing to |pipefd|, and only then creates |rescan_pipe|. So if
HPStopHotPluggables() is called in between, it'll end up doing nothing,
letting the hotplug thread sit infinitely long on the read() call
meanwhile the main daemon thread is shutting down.

The proposed fix is to create |rescan_pipe| in advance, so that the
shutdown logic is guaranteed to be able to write a byte to it, notifying
the background thread about the shutdown regardless of when it happens.
@emaxx-google emaxx-google force-pushed the libusb-rescan-no-pipe-race branch from 0174793 to dad51a7 Compare April 13, 2024 19:18
@emaxx-google
Copy link
Contributor Author

Please rebase

done

@LudovicRousseau
Copy link
Owner

Merged.
Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants