01-23-2010, 03:19 AM
Problem
A deadlock may occur in core.cpp:g_EmuThread->WaitForDeath(), and possibly also in cpuThread->WaitForDeath() and emuThreadGoing.MsgWait().
Problem as seen by the user
Stopping a running game, or closing Dolphin, locks the Dolphin process. Leading the user to the inconvenience of opening the Task Manager and closing the process.
Likelihood of occurrence
From my tests the deadlock seemed fairly likely to occur. Depending on race conditions when ending the video thread, more precisely if execution in the video loop is before one of the deadlock points when the thread is asked to close.
And example of a condition that caused several repeated deadlocks in r4771 was running a GC or Wii game with a gamepad connected, but not have the console window open. If the console window was open, or if no gamepad was connected, no deadlock occurred (for five or ten tests).
In r4922 g_EmuThread->WaitForDeath() would deadlock a few times even when the console window is open, and a gamepad is connected. However, it would sometimes not deadlock even when a gamepad was connected.
So my conclusion is that the deadlock still exists. Possibly at the same point as in many revisions back. And that it's fairly likely to occur. So until someone with the expertise and inclination can find the root cause of the the deadlock problem, explain the faulting code, and fix it so that there is no chance of a deadlock anywhere in the video thread, it's my recommendation that this hack is applied.
Solution
End all threads before stopping the core. It's reliable because a timer, that never locks any thread, waits for the video thread to signal that it's gone to sleep.
Comments about the implementation
The message to the Core is sent through a PostMessage. A better solution would be to add an exported DLL function that can send the message.
Why is it a hack?
The proper way to fix the deadlock is to find the code in the video thread that the thread deadlocks at, and fix that. So that WaitForSingleObject doesn't lock the thread it's waiting for to end.
The deadlock problem in general
I'm not sure why this kind of problem occurs. From the Wiimote thread deadlock that was mentioned in r2083 I would draw the conclusion that the deadlock occurs when objects or functions declared or defined in machine-code in another thread is called. In the r2083 example calls to objects in the 'frame' class, declared and defined in the main thread, would deadlock, if the main thread was in a WaitForSingleObject loop, a narrow while-loop or in a sleep() loop.
The reason the deadlock doesn't always occur is because there is only one or a few points in the video thread where a deadlock will occur. So the deadlock will only occur if the loop is between the beginning of the loop (in Fifo.cpp:Fifo_EnterLoop in this case) and the place that deadlocks in the case that the main thread is locked in the WaitForSingleObject loop. If the execution is between directly after the last line of code that deadlocks and the end of the loop the deadlock will never occur. For example in the r2083 deadlock problem all or some of the 'frame->' object and function calls deadlocked. So if the loop was after the last of those and the end of the loop, a deadlock didn't occurred.
It's probably not specifically the WaitForSingleObject loop that cause the deadlock, the same thing will probably occur if the thread that calls WaitForSingleObject is locked in a narrow while-loop instead of in the WaitForSingleObject loop. The problem is then that execution in the thread (in this case the video thread) stalls at some point (literally stops at a certain line of machine-code), if execution in another thread is locked in a narrow loop or sleeping. An example of that is the problem with the Wiimote thread, fixed through a hack in r2083. A sleep() or while loop in the main thread would halt the g_pReadThread thread in the same place as the WaitForSingleObject loop did. In this case it was one or a few specific function or object calls where it stalled at, specifically some or all calls to functions or object in the 'frame' class, for example frame->m_GaugeBattery or frame->DoRecordMovement() or one of the other 'frame->' objects or functions in ReadWiimote.cpp:ReadWiimote(). I could not find a way to fix the root cause of this problem so that the frame-> objects could be called without a deadlock. The only way I could fix it was to make sure this deadlock could not occur, with the means of a timer similar to this solution.
Resources about deadlocks
MsgWaitForMultipleObjects Function (Windows)
MsgWaitForMultipleObjects should be used instead of WaitForSingleObject if the calling thread has a message loop.
Detecting Potential Deadlocks: Uncovering problems before they occur
This article describes a Deadlock Detection Application (which I could not find on the page unfortunately)
How to create a proper solution
If there is no downside to it g_EmuThread->WaitForDeath() could be replaced with g_EmuThread->MsgWait(). This would fix deadlocks that are related to the message loop in the calling thread.
A deadlock may occur in core.cpp:g_EmuThread->WaitForDeath(), and possibly also in cpuThread->WaitForDeath() and emuThreadGoing.MsgWait().
Problem as seen by the user
Stopping a running game, or closing Dolphin, locks the Dolphin process. Leading the user to the inconvenience of opening the Task Manager and closing the process.
Likelihood of occurrence
From my tests the deadlock seemed fairly likely to occur. Depending on race conditions when ending the video thread, more precisely if execution in the video loop is before one of the deadlock points when the thread is asked to close.
And example of a condition that caused several repeated deadlocks in r4771 was running a GC or Wii game with a gamepad connected, but not have the console window open. If the console window was open, or if no gamepad was connected, no deadlock occurred (for five or ten tests).
In r4922 g_EmuThread->WaitForDeath() would deadlock a few times even when the console window is open, and a gamepad is connected. However, it would sometimes not deadlock even when a gamepad was connected.
So my conclusion is that the deadlock still exists. Possibly at the same point as in many revisions back. And that it's fairly likely to occur. So until someone with the expertise and inclination can find the root cause of the the deadlock problem, explain the faulting code, and fix it so that there is no chance of a deadlock anywhere in the video thread, it's my recommendation that this hack is applied.
Solution
End all threads before stopping the core. It's reliable because a timer, that never locks any thread, waits for the video thread to signal that it's gone to sleep.
Comments about the implementation
The message to the Core is sent through a PostMessage. A better solution would be to add an exported DLL function that can send the message.
Why is it a hack?
The proper way to fix the deadlock is to find the code in the video thread that the thread deadlocks at, and fix that. So that WaitForSingleObject doesn't lock the thread it's waiting for to end.
The deadlock problem in general
I'm not sure why this kind of problem occurs. From the Wiimote thread deadlock that was mentioned in r2083 I would draw the conclusion that the deadlock occurs when objects or functions declared or defined in machine-code in another thread is called. In the r2083 example calls to objects in the 'frame' class, declared and defined in the main thread, would deadlock, if the main thread was in a WaitForSingleObject loop, a narrow while-loop or in a sleep() loop.
The reason the deadlock doesn't always occur is because there is only one or a few points in the video thread where a deadlock will occur. So the deadlock will only occur if the loop is between the beginning of the loop (in Fifo.cpp:Fifo_EnterLoop in this case) and the place that deadlocks in the case that the main thread is locked in the WaitForSingleObject loop. If the execution is between directly after the last line of code that deadlocks and the end of the loop the deadlock will never occur. For example in the r2083 deadlock problem all or some of the 'frame->' object and function calls deadlocked. So if the loop was after the last of those and the end of the loop, a deadlock didn't occurred.
It's probably not specifically the WaitForSingleObject loop that cause the deadlock, the same thing will probably occur if the thread that calls WaitForSingleObject is locked in a narrow while-loop instead of in the WaitForSingleObject loop. The problem is then that execution in the thread (in this case the video thread) stalls at some point (literally stops at a certain line of machine-code), if execution in another thread is locked in a narrow loop or sleeping. An example of that is the problem with the Wiimote thread, fixed through a hack in r2083. A sleep() or while loop in the main thread would halt the g_pReadThread thread in the same place as the WaitForSingleObject loop did. In this case it was one or a few specific function or object calls where it stalled at, specifically some or all calls to functions or object in the 'frame' class, for example frame->m_GaugeBattery or frame->DoRecordMovement() or one of the other 'frame->' objects or functions in ReadWiimote.cpp:ReadWiimote(). I could not find a way to fix the root cause of this problem so that the frame-> objects could be called without a deadlock. The only way I could fix it was to make sure this deadlock could not occur, with the means of a timer similar to this solution.
Resources about deadlocks
MsgWaitForMultipleObjects Function (Windows)
MsgWaitForMultipleObjects should be used instead of WaitForSingleObject if the calling thread has a message loop.
Detecting Potential Deadlocks: Uncovering problems before they occur
This article describes a Deadlock Detection Application (which I could not find on the page unfortunately)
How to create a proper solution
If there is no downside to it g_EmuThread->WaitForDeath() could be replaced with g_EmuThread->MsgWait(). This would fix deadlocks that are related to the message loop in the calling thread.