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

InputCommon/WGInput: Handle add/remove events on separate thread to prevent deadlocks. #12508

Merged
merged 1 commit into from Jan 18, 2024

Conversation

AdmiralCurtiss
Copy link
Contributor

@AdmiralCurtiss AdmiralCurtiss commented Jan 17, 2024

In particular this is triggered when running Dolphin with the Steam overlay.

This should be a more formally correct alternative to #12504. Built on top of #12503 so merge that first please.

@@ -668,6 +684,36 @@ static void RemoveDevice(const WGI::RawGameController& raw_game_controller)
// (H is the lambda)
#pragma warning(disable : 4265)

static void HandleAddRemoveEvent(AddRemoveEvent evt)
Copy link
Contributor

@Filoppi Filoppi Jan 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this will be run in a separate thread, but when calling this:

s_event_added = WGI::RawGameController::RawGameControllerAdded(
  [](auto&&, WGI::RawGameController raw_game_controller) {
    s_device_add_remove_queue.EmplaceItem(
        AddRemoveEvent{AddRemoveEventType::Add, std::move(raw_game_controller)});
        });

is it going to wait for this thread to have run the event, or will it be delayed to later? If it's done "inline" then there's still a risk of deadlock.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing should block until completion here. WGI::RawGameController::RawGameControllerAdded() registers a lambda to be called every time a new RawGameController is noticed by WGI. s_device_add_remove_queue.EmplaceItem() registers an item to be executed in the WorkQueueThread's worker thread and does not wait for that item to be processed or anything like that -- it may block very shortly while the item is added into the queue if another thread is also adding to the queue or while the worker thread is taking an item from the queue to to be executed, but that can never cause a deadlock.

…revent deadlocks.

In particular this is triggered when running Dolphin with the Steam overlay.
@AdmiralCurtiss AdmiralCurtiss merged commit db43f90 into dolphin-emu:master Jan 18, 2024
11 checks passed
@AdmiralCurtiss AdmiralCurtiss deleted the wgi-deadlock branch January 18, 2024 09:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants