This boils down to a CriticalSection usage bug. The hanging that we see in the applet is caused by our rendering thread coming to a complete halt as it waits for a CriticalSection (surfaceLock) to be released which will never be released.
The problem is that we have already released the CriticalSection ... twice.
In DDrawSurface::GetDC(), we ask a ddraw surface for an HDC object that we can use to render with GDI. Like all calls in DDrawSurface, we first grab the surfaceLock CriticalSection, to ensure that no other thread messes with the surface while we are using it. If the call to GetDC on the surface fails, then we release the CriticalSection because the calling code should not call ReleaseDC since the HDC is NULL (and therefore invalid).
This works for most of the cases that call GetDC; all of the functions in Win32Renderer noop when the HDC is NULL and simply return without calling ReleaseDC (thus there is exactly one Enter and one Leave for the surfaceLock CriticalSection).
However, there is code in Win32BlitLoops.cpp that does not behave this convention. When we are in a multimon situation and require a Blt to the secondary screen, then we call GetDC for the src and destination surfaces
(in this case, the src is a ddraw offscreen surface and the dest is the onscreen window). But we do not check for failure of GetDC; we simply proceed with the GDI BitBlt() operations and then eventually call ReleaseDC() for both the src and destination. This means that if we ever fail the call to GetDC and return NULL, we will release the surfaceLock once during the failure of GetDC, and then release it again during the later (incorrect) call to ReleaseDC. If a CriticalSection is released more than once, the results are unpredictable. From the MSDN docs: "If a thread calls LeaveCriticalSection when it does not have ownership ... an error occurs that may cause another thread using EnterCriticalSection to wait indefinitely."
The fix is easy: make the code in Win32BlitLoops behave by:
- checking for a NULL HDC for both the src and dst surfaces
- if either are NULL, release the one that is non-NULL, and return from the function (noop; we cannot hope to actually do a Gdi blit here since we did not get the structures necessary for performing the operation).