[PATCH v3 0/2] MR10889: ole32: Validate cached clipboard window handle in get_clipbrd_window.
The OLE clipboard caches a per-process window handle. When the STA thread that created this window terminates, the window is destroyed but the stale handle remains cached. Subsequent OleSetClipboard calls use this stale handle, causing OpenClipboard to fail with CLIPBRD_E_CANT_OPEN. Check whether the cached window is still valid using IsWindow() before returning it. If the window has been destroyed, clear the cache so a fresh window is created on the current thread. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=59519 -- v3: ole32: Validate cached clipboard window handle in get_clipbrd_window. https://gitlab.winehq.org/wine/wine/-/merge_requests/10889
From: Aaron Yourk <ayourk@gmail.com> Test that OleSetClipboard succeeds when called from a second STA thread after the first thread (which created the cached clipboard window) has exited. Currently this fails because the stale window handle is never invalidated. Based on the bug reproducer source. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=59519 --- dlls/ole32/tests/clipboard.c | 92 ++++++++++++++++++++++++++++++++++++ 1 file changed, 92 insertions(+) diff --git a/dlls/ole32/tests/clipboard.c b/dlls/ole32/tests/clipboard.c index 50e4e8c27a3..dedf23f629c 100644 --- a/dlls/ole32/tests/clipboard.c +++ b/dlls/ole32/tests/clipboard.c @@ -1925,6 +1925,97 @@ static void test_get_clipboard_locked(void) OleUninitialize(); } +struct sta_clipboard_thread_data +{ + const char *text; + HRESULT hr_set; +}; + +static DWORD CALLBACK sta_clipboard_set_thread(void *arg) +{ + struct sta_clipboard_thread_data *data = arg; + IDataObject *dataobj; + HRESULT hr; + + hr = OleInitialize(NULL); + if (FAILED(hr)) + { + data->hr_set = hr; + return 1; + } + + hr = DataObjectImpl_CreateText(data->text, &dataobj); + ok(hr == S_OK, "DataObjectImpl_CreateText returned %#lx\n", hr); + + hr = OleSetClipboard(dataobj); + data->hr_set = hr; + IDataObject_Release(dataobj); + + if (SUCCEEDED(hr)) + OleFlushClipboard(); + + OleUninitialize(); + return 0; +} + +static void test_sta_clipboard_reuse(void) +{ + struct sta_clipboard_thread_data data; + HANDLE thread, hdata; + HRESULT hr; + char *text; + DWORD ret; + + hr = OleInitialize(NULL); + ok(hr == S_OK, "OleInitialize returned %#lx\n", hr); + + /* First STA thread sets clipboard to "first". */ + data.text = "first"; + data.hr_set = E_FAIL; + thread = CreateThread(NULL, 0, sta_clipboard_set_thread, &data, 0, NULL); + ok(thread != NULL, "CreateThread failed (%ld)\n", GetLastError()); + ret = WaitForSingleObject(thread, 5000); + ok(ret == WAIT_OBJECT_0, "WaitForSingleObject returned %#lx\n", ret); + CloseHandle(thread); + ok(data.hr_set == S_OK, "first OleSetClipboard returned %#lx\n", data.hr_set); + + ok(OpenClipboard(NULL), "OpenClipboard failed\n"); + hdata = GetClipboardData(CF_TEXT); + ok(hdata != NULL, "GetClipboardData returned NULL\n"); + if (hdata) + { + text = GlobalLock(hdata); + ok(!strcmp(text, "first"), "expected \"first\", got \"%s\"\n", text); + GlobalUnlock(hdata); + } + CloseClipboard(); + + /* Second STA thread sets clipboard to "second". The first thread's + OLE clipboard window has been destroyed, but Wine caches the stale + HWND and the second OleSetClipboard fails with CLIPBRD_E_CANT_OPEN. */ + data.text = "second"; + data.hr_set = E_FAIL; + thread = CreateThread(NULL, 0, sta_clipboard_set_thread, &data, 0, NULL); + ok(thread != NULL, "CreateThread failed (%ld)\n", GetLastError()); + ret = WaitForSingleObject(thread, 5000); + ok(ret == WAIT_OBJECT_0, "WaitForSingleObject returned %#lx\n", ret); + CloseHandle(thread); + todo_wine ok(data.hr_set == S_OK, "second OleSetClipboard returned %#lx\n", data.hr_set); + + ok(OpenClipboard(NULL), "OpenClipboard failed\n"); + hdata = GetClipboardData(CF_TEXT); + ok(hdata != NULL, "GetClipboardData returned NULL\n"); + if (hdata) + { + text = GlobalLock(hdata); + todo_wine ok(!strcmp(text, "second"), "expected \"second\", got \"%s\"\n", text); + GlobalUnlock(hdata); + } + CloseClipboard(); + + OleUninitialize(); +} + START_TEST(clipboard) { test_get_clipboard_uninitialized(); @@ -1935,5 +2026,6 @@ START_TEST(clipboard) test_nonole_clipboard(); test_getdatahere(); test_multithreaded_clipboard(); + test_sta_clipboard_reuse(); test_get_clipboard_locked(); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10889
From: Aaron Yourk <ayourk@gmail.com> The OLE clipboard caches a per-process window handle. When the STA thread that created this window terminates, the window is destroyed but the stale handle remains cached. Subsequent OleSetClipboard calls use this stale handle, causing OpenClipboard to fail with CLIPBRD_E_CANT_OPEN. Check whether the cached window is still valid using IsWindow() before returning it. If the window has been destroyed, clear the cache so a fresh window is created on the current thread. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=59519 --- dlls/ole32/clipboard.c | 3 +++ dlls/ole32/tests/clipboard.c | 4 ++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/dlls/ole32/clipboard.c b/dlls/ole32/clipboard.c index c54608985fb..749dd03c3e6 100644 --- a/dlls/ole32/clipboard.c +++ b/dlls/ole32/clipboard.c @@ -1899,6 +1899,9 @@ static HWND create_clipbrd_window(void); */ static inline HRESULT get_clipbrd_window(ole_clipbrd *clipbrd, HWND *wnd) { + if ( clipbrd->window && !IsWindow(clipbrd->window) ) + clipbrd->window = NULL; + if ( !clipbrd->window ) clipbrd->window = create_clipbrd_window(); diff --git a/dlls/ole32/tests/clipboard.c b/dlls/ole32/tests/clipboard.c index dedf23f629c..043e73daf38 100644 --- a/dlls/ole32/tests/clipboard.c +++ b/dlls/ole32/tests/clipboard.c @@ -2000,7 +2000,7 @@ static void test_sta_clipboard_reuse(void) ret = WaitForSingleObject(thread, 5000); ok(ret == WAIT_OBJECT_0, "WaitForSingleObject returned %#lx\n", ret); CloseHandle(thread); - todo_wine ok(data.hr_set == S_OK, "second OleSetClipboard returned %#lx\n", data.hr_set); + ok(data.hr_set == S_OK, "second OleSetClipboard returned %#lx\n", data.hr_set); ok(OpenClipboard(NULL), "OpenClipboard failed\n"); hdata = GetClipboardData(CF_TEXT); @@ -2008,7 +2008,7 @@ static void test_sta_clipboard_reuse(void) if (hdata) { text = GlobalLock(hdata); - todo_wine ok(!strcmp(text, "second"), "expected \"second\", got \"%s\"\n", text); + ok(!strcmp(text, "second"), "expected \"second\", got \"%s\"\n", text); GlobalUnlock(hdata); } CloseClipboard(); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10889
Huw Davies (@huw) commented about dlls/ole32/tests/clipboard.c:
+ ok(data.hr_set == S_OK, "first OleSetClipboard returned %#lx\n", data.hr_set); + + ok(OpenClipboard(NULL), "OpenClipboard failed\n"); + hdata = GetClipboardData(CF_TEXT); + ok(hdata != NULL, "GetClipboardData returned NULL\n"); + if (hdata) + { + text = GlobalLock(hdata); + ok(!strcmp(text, "first"), "expected \"first\", got \"%s\"\n", text); + GlobalUnlock(hdata); + } + CloseClipboard(); + + /* Second STA thread sets clipboard to "second". The first thread's + OLE clipboard window has been destroyed, but Wine caches the stale + HWND and the second OleSetClipboard fails with CLIPBRD_E_CANT_OPEN. */ This MR broadly looks ok, but could you remove this comment - it's not going to be true after your fix.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/10889#note_140540
participants (3)
-
Aaron Yourk -
Aaron Yourk (＠ayourk)
-
Huw Davies (＠huw)