[PATCH v3 1/2] strmbase: Detach the window from the parent before destroying it.
Fixes a regression introduced by 3b5198c8283d891095612c1001edb5e5788d6059. Media Player Classic deadlocks when the window is destroyed, because DestroyWindow will notify the parent, which is on a different thread and waiting on a signal from the filter's thread. On Windows, when WM_DESTROY is received, it's already detached from the parent. However, manually sending a WM_CLOSE to the window does not detach it (it actually doesn't seem to destroy it, either) which means the detach process is not done in WM_CLOSE in Windows. Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- Since all of this is implementation detail that I don't think we should go much into, this new version of the patch simply detaches the window before we send the WM_CLOSE ourselves, which does make the app happy and closely resembles Windows behavior. dlls/strmbase/window.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/dlls/strmbase/window.c b/dlls/strmbase/window.c index 07a12ec..0a2ef9f 100644 --- a/dlls/strmbase/window.c +++ b/dlls/strmbase/window.c @@ -139,6 +139,11 @@ HRESULT WINAPI BaseWindowImpl_DoneWithWindow(BaseWindow *This) if (!This->hWnd) return S_OK; + /* Media Player Classic deadlocks when the parent, which is on a different thread, + is notified before the destruction. Windows itself seems to detach the window + from the parent prior to destroying it, so we also do that here as well. */ + IVideoWindow_put_Owner(&impl_from_BaseWindow(This)->IVideoWindow_iface, (OAHWND)NULL); + SendMessageW(This->hWnd, WM_CLOSE, 0, 0); This->hWnd = NULL; -- 2.21.0
Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/quartz/tests/filtergraph.c | 44 ++++++++++++++++++++++++++++++++- 1 file changed, 43 insertions(+), 1 deletion(-) diff --git a/dlls/quartz/tests/filtergraph.c b/dlls/quartz/tests/filtergraph.c index ca45031..95b2082 100644 --- a/dlls/quartz/tests/filtergraph.c +++ b/dlls/quartz/tests/filtergraph.c @@ -4232,16 +4232,39 @@ static HWND get_renderer_hwnd(IFilterGraph2 *graph) return hwnd; } +static LRESULT CALLBACK parent_wndproc(HWND hwnd, UINT msg, WPARAM wParam, LPARAM lParam) +{ + switch (msg) + { + case WM_PARENTNOTIFY: + if (LOWORD(wParam) == WM_DESTROY) + ok(0, "Received WM_PARENTNOTIFY with WM_DESTROY.\n"); + break; + } + return DefWindowProcA(hwnd, msg, wParam, lParam); +} + static void test_window_threading(void) { WCHAR *filename = load_resource(avifile); IFilterGraph2 *graph = create_graph(); + IVideoWindow *videownd; + WNDCLASSA cls = { 0 }; + HWND hwnd, parent; HRESULT hr; DWORD tid; ULONG ref; - HWND hwnd; BOOL ret; + cls.lpfnWndProc = parent_wndproc; + cls.hInstance = GetModuleHandleA(NULL); + cls.hCursor = LoadCursorA(0, (const char*)IDC_ARROW); + cls.lpszClassName = "TestParent"; + RegisterClassA(&cls); + + parent = CreateWindowExA(0, "TestParent", NULL, WS_OVERLAPPEDWINDOW, 50, 50, 150, 150, NULL, NULL, cls.hInstance, NULL); + ok(parent != NULL, "Failed to create parent window.\n"); + hr = IFilterGraph2_RenderFile(graph, filename, NULL); if (FAILED(hr)) { @@ -4259,6 +4282,14 @@ static void test_window_threading(void) /* The thread should be processing messages, or this will hang. */ SendMessageA(hwnd, WM_NULL, 0, 0); + + /* The WM_DESTROY notification is not sent to the owner, even if the style is off */ + hr = IFilterGraph2_QueryInterface(graph, &IID_IVideoWindow, (void**)&videownd); + ok(hr == S_OK, "Got hr %#x.\n", hr); + hr = IVideoWindow_put_Owner(videownd, (OAHWND)parent); + ok(hr == S_OK, "Got hr %#x.\n", hr); + IVideoWindow_Release(videownd); + ok(!(GetWindowLongW(hwnd, GWL_EXSTYLE) & WS_EX_NOPARENTNOTIFY), "Window has WS_EX_NOPARENTNOTIFY.\n"); } else skip("Could not find renderer window.\n"); @@ -4277,6 +4308,14 @@ static void test_window_threading(void) { tid = GetWindowThreadProcessId(hwnd, NULL); ok(tid == GetCurrentThreadId(), "Window should be created on main thread.\n"); + + /* The WM_DESTROY notification is not sent to the owner, even if the style is off */ + hr = IFilterGraph2_QueryInterface(graph, &IID_IVideoWindow, (void**)&videownd); + ok(hr == S_OK, "Got hr %#x.\n", hr); + hr = IVideoWindow_put_Owner(videownd, (OAHWND)parent); + ok(hr == S_OK, "Got hr %#x.\n", hr); + IVideoWindow_Release(videownd); + ok(!(GetWindowLongW(hwnd, GWL_EXSTYLE) & WS_EX_NOPARENTNOTIFY), "Window has WS_EX_NOPARENTNOTIFY.\n"); } else skip("Could not find renderer window.\n"); @@ -4285,6 +4324,9 @@ static void test_window_threading(void) ok(!ref, "Got outstanding refcount %d.\n", ref); ret = DeleteFileW(filename); ok(ret, "Failed to delete file, error %u.\n", GetLastError()); + + DestroyWindow(parent); + UnregisterClassA("TestParent", cls.hInstance); } START_TEST(filtergraph) -- 2.21.0
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=63144 Your paranoid android. === w1064v1809_he (32 bit report) === quartz: filtergraph.c:527: Test failed: didn't get EOS filtergraph.c:532: Test failed: expected 2ccdba, got 0 filtergraph.c:3932: Test failed: Expected about 1334ms, got d81080. filtergraph.c:3937: Test failed: Expected about 1334ms, got d83790.
participants (2)
-
Gabriel Ivăncescu -
Marvin