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

DolphinQt: Prevent deadlock when exiting the NetPlay Session Browser dialog #9028

Merged

Conversation

cristian64
Copy link
Contributor

@cristian64 cristian64 commented Aug 14, 2020

It was possible to enter a deadlock when the NetPlay Session Browser dialog was closed while a refresh was still ongoing.

Reproduction steps:

  • Launch Dolphin.
  • Open the NetPlay Session Browser dialog via Tools > Browse NetPlay Sessions....
  • Press ESC as soon as the dialog pops up, so it is dismissed straight away.
  • Dolphin will (potentially) deadlock.

Stacktrace (Release build):

Thread 1 (Thread 0x7f3f319e6080 (LWP 4361)):
#0  0x00007fa6be274d2d in __GI___pthread_timedjoin_ex (threadid=140352865023744, thread_return=0x0, abstime=0x0, block=<optimised out>) at pthread_join_common.c:89
#1  0x00007fa6bdfa0933 in std::thread::join() () at /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#2  0x000055c4f2f996ea in NetPlayBrowser::~NetPlayBrowser() ()
#3  0x000055c4f2f99799 in NetPlayBrowser::~NetPlayBrowser() ()
#4  0x00007fa6c50acf31 in QDialog::exec() () at /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
#5  0x000055c4f2e993d2 in MainWindow::ShowNetPlayBrowser() ()
...

Thread 14 (Thread 0x7f8d41ffb700 (LWP 27323)):
#0  0x00007f8d7f8fb9f3 in futex_wait_cancelable (private=<optimised out>, expected=0, futex_word=0x7f8d41ff1430) at ../sysdeps/unix/sysv/linux/futex-internal.h:88
#1  0x00007f8d7f8fb9f3 in __pthread_cond_wait_common (abstime=0x0, mutex=0x7f8d41ff1438, cond=0x7f8d41ff1408) at pthread_cond_wait.c:502
#2  0x00007f8d7f8fb9f3 in __pthread_cond_wait (cond=0x7f8d41ff1408, mutex=0x7f8d41ff1438) at pthread_cond_wait.c:655
#3  0x00007f8d7f61c8bc in std::condition_variable::wait(std::unique_lock<std::mutex>&) () at /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#4  0x000055b844a9ea5f in Common::Event::Wait() [clone .part.75] ()
#5  0x000055b844aa2710 in NetPlayBrowser::RefreshLoop() ()
#6  0x00007f8d7f6226df in  () at /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#7  0x00007f8d7f8f56db in start_thread (arg=0x7f8d41ffb700) at pthread_create.c:463
#8  0x00007f8d7eef7a3f in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95
...

When the NetPlayBrowser::~NetPlayBrowser() is invoked, it attempts to cancel the network thread, and waits for it to return control. However, if a refresh request was still in progress, the network thread might still attempt to update some widgets via RunOnObject(). Therefore, Qt's event loop remains busy, waiting for the network thread to yield control, and the network thread is waiting for Qt to run a lambda in the object.

@cristian64 cristian64 changed the title Prevent deadlock when exiting the NetPlay Session Browser dialog DolphinQt: Prevent deadlock when exiting the NetPlay Session Browser dialog Aug 14, 2020
… widgets, to prevent multiple refresh requests on change.
@cristian64 cristian64 force-pushed the netplaybrowser_deadlock_on_exit branch from a41dfcc to 8ac92dc Compare August 15, 2020 21:32
@cristian64
Copy link
Contributor Author

cristian64 commented Aug 15, 2020

Updated to put the changes regarding RestoreSettings() in its own commit.

The function is now (still) being invoked from the constructor, before ConnectWidgets(), to avoid triggering Refresh() multiple times as widgets will change their values and emit their signals.

@cristian64 cristian64 force-pushed the netplaybrowser_deadlock_on_exit branch from 8ac92dc to f5241ba Compare August 16, 2020 02:08
@cristian64 cristian64 force-pushed the netplaybrowser_deadlock_on_exit branch from f5241ba to 2af2ac1 Compare August 16, 2020 10:36
@cristian64 cristian64 force-pushed the netplaybrowser_deadlock_on_exit branch from 2af2ac1 to ddeb223 Compare August 16, 2020 11:33
@lioncash lioncash merged commit 2c5920d into dolphin-emu:master Aug 16, 2020
10 checks passed
@cristian64 cristian64 deleted the netplaybrowser_deadlock_on_exit branch November 22, 2020 20:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants