[PATCH v2 0/2] MR10245: ole32: Relax COM initialization check in RegisterDragDrop.
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=59084 -- v2: ole32: Relax COM initialization check in RegisterDragDrop. ole32/tests: Add more RegisterDragDrop tests after mismatched cleanup. https://gitlab.winehq.org/wine/wine/-/merge_requests/10245
From: Akihiro Sagawa <sagawa.aki@gmail.com> --- dlls/ole32/tests/dragdrop.c | 68 +++++++++++++++++++++++++++++++++++++ 1 file changed, 68 insertions(+) diff --git a/dlls/ole32/tests/dragdrop.c b/dlls/ole32/tests/dragdrop.c index 1c7dd07ea93..a71884b771b 100644 --- a/dlls/ole32/tests/dragdrop.c +++ b/dlls/ole32/tests/dragdrop.c @@ -265,6 +265,23 @@ static int test_reentrance; /* helper macros to make tests a bit leaner */ #define ok_ole_success(hr, func) ok(hr == S_OK, func " failed with error %#08lx\n", hr) +static void run_child_test(const char *name) +{ + char path_name[MAX_PATH]; + PROCESS_INFORMATION info; + STARTUPINFOA startup; + char **argv; + + winetest_get_mainargs(&argv); + + memset(&startup, 0, sizeof(startup)); + startup.cb = sizeof(startup); + sprintf(path_name, "\"%s\" dragdrop %s", argv[0], name); + ok(CreateProcessA( NULL, path_name, NULL, NULL, FALSE, 0, NULL, NULL, &startup, &info), + "CreateProcess failed.\n" ); + wait_child_process(&info); +} + static HRESULT WINAPI DropTarget_QueryInterface(IDropTarget* iface, REFIID riid, void** ppvObject) { @@ -753,10 +770,61 @@ static void test_DoDragDrop(void) DestroyWindow(hwnd); } +static void test_mismatched_cleanup(void) +{ + HRESULT hr; + HWND hwnd; + + hr = OleInitialize(NULL); + ok(hr == S_OK, "OleInitialize failed, got %#lx\n", hr); + + /* Mismatched cleanup (left dangling window class registration) */ + CoUninitialize(); + + /* Now, RegisterDragDrop call fails with another error code */ + hwnd = CreateWindowExA(WS_EX_TOPMOST, "WineOleTestClass", "Test", 0, + CW_USEDEFAULT, CW_USEDEFAULT, 100, 100, NULL, + NULL, NULL, NULL); + ok(IsWindow(hwnd), "failed to create window\n"); + + hr = RegisterDragDrop(hwnd, &DropTarget); + todo_wine ok(hr == CO_E_NOTINITIALIZED, "expected CO_E_NOTINITIALIZED, got %#lx\n", hr); + + DestroyWindow(hwnd); + + /* However, RegisterDragDrop succeeds after creating MTA */ + hr = CoInitializeEx(NULL, COINIT_MULTITHREADED); + ok(hr == S_OK, "CoInitializeEx failed, got %#lx\n", hr); + + hwnd = CreateWindowExA(WS_EX_TOPMOST, "WineOleTestClass", "Test", 0, + CW_USEDEFAULT, CW_USEDEFAULT, 100, 100, NULL, + NULL, NULL, NULL); + ok(IsWindow(hwnd), "failed to create window\n"); + + hr = RegisterDragDrop(hwnd, &DropTarget); + todo_wine ok(hr == S_OK, "expected S_OK, got %#lx\n", hr); + + DestroyWindow(hwnd); + + CoUninitialize(); +} + START_TEST(dragdrop) { + int argc; + char **argv; + register_dummy_class(); + argc = winetest_get_mainargs( &argv ); + if (argc > 2) + { + if (!strcmp(argv[2], "mismatched_cleanup")) + test_mismatched_cleanup(); + return; + } + test_Register_Revoke(); test_DoDragDrop(); + run_child_test("mismatched_cleanup"); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10245
From: Akihiro Sagawa <sagawa.aki@gmail.com> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=59084 --- dlls/ole32/compobj_private.h | 5 ----- dlls/ole32/ole2.c | 15 +++++++++++++-- dlls/ole32/tests/dragdrop.c | 4 ++-- 3 files changed, 15 insertions(+), 9 deletions(-) diff --git a/dlls/ole32/compobj_private.h b/dlls/ole32/compobj_private.h index 0708a3d7782..f21c0ea099d 100644 --- a/dlls/ole32/compobj_private.h +++ b/dlls/ole32/compobj_private.h @@ -89,11 +89,6 @@ static inline struct oletls *COM_CurrentInfo(void) return NtCurrentTeb()->ReservedForOle; } -static inline struct apartment * COM_CurrentApt(void) -{ - return COM_CurrentInfo()->apt; -} - #define CHARS_IN_GUID 39 /* including NULL */ /* from dlldata.c */ diff --git a/dlls/ole32/ole2.c b/dlls/ole32/ole2.c index b37b5b0e387..6847fa2afb4 100644 --- a/dlls/ole32/ole2.c +++ b/dlls/ole32/ole2.c @@ -50,6 +50,9 @@ WINE_DEFAULT_DEBUG_CHANNEL(ole); WINE_DECLARE_DEBUG_CHANNEL(accel); +/* Combase exports */ +BOOL WINAPI InternalIsProcessInitialized(void); + /****************************************************************************** * These are static/global variables and internal data structures that the * OLE module uses to maintain its state. @@ -536,6 +539,7 @@ static IDropTarget* get_droptarget_pointer(HWND hwnd) */ HRESULT WINAPI RegisterDragDrop(HWND hwnd, LPDROPTARGET pDropTarget) { + WNDCLASSW wndClass; DWORD pid = 0; HRESULT hr; IStream *stream; @@ -544,9 +548,10 @@ HRESULT WINAPI RegisterDragDrop(HWND hwnd, LPDROPTARGET pDropTarget) TRACE("(%p,%p)\n", hwnd, pDropTarget); - if (!COM_CurrentApt()) + /* did we call OleInitialize before? */ + if (!GetClassInfoW(NULL, OLEDD_DRAGTRACKERCLASS, &wndClass)) { - ERR("COM not initialized\n"); + ERR("OleInitialize not called\n"); return E_OUTOFMEMORY; } @@ -559,6 +564,12 @@ HRESULT WINAPI RegisterDragDrop(HWND hwnd, LPDROPTARGET pDropTarget) return DRAGDROP_E_INVALIDHWND; } + if (!InternalIsProcessInitialized()) + { + ERR("COM not initialized\n"); + return CO_E_NOTINITIALIZED; + } + /* block register for other processes windows */ GetWindowThreadProcessId(hwnd, &pid); if (pid != GetCurrentProcessId()) diff --git a/dlls/ole32/tests/dragdrop.c b/dlls/ole32/tests/dragdrop.c index a71884b771b..441b11a9595 100644 --- a/dlls/ole32/tests/dragdrop.c +++ b/dlls/ole32/tests/dragdrop.c @@ -788,7 +788,7 @@ static void test_mismatched_cleanup(void) ok(IsWindow(hwnd), "failed to create window\n"); hr = RegisterDragDrop(hwnd, &DropTarget); - todo_wine ok(hr == CO_E_NOTINITIALIZED, "expected CO_E_NOTINITIALIZED, got %#lx\n", hr); + ok(hr == CO_E_NOTINITIALIZED, "expected CO_E_NOTINITIALIZED, got %#lx\n", hr); DestroyWindow(hwnd); @@ -802,7 +802,7 @@ static void test_mismatched_cleanup(void) ok(IsWindow(hwnd), "failed to create window\n"); hr = RegisterDragDrop(hwnd, &DropTarget); - todo_wine ok(hr == S_OK, "expected S_OK, got %#lx\n", hr); + ok(hr == S_OK, "expected S_OK, got %#lx\n", hr); DestroyWindow(hwnd); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10245
On Wed Mar 4 14:00:39 2026 +0000, Nikolay Sivov wrote:
This does not look good in my opinion. To me it looks like checking for some non obvious side effect. If you need to check current-or-mta case, you can use InternalIsProcessInitialized() from combase. We already use it in ole32. The RegisterDragDrop function requires OleInitialize to be called beforehand, and this check ensures that the resources initialized by OleInitialize are still allocated and have not been released.
In the v2 patch, I added an InternalIsProcessInitialized call to explicitly verify that COM have been initialized by OleInitialize or CoInitializeEx. Thanks for reviewing. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10245#note_131534
On Sat Mar 7 10:11:55 2026 +0000, Akihiro Sagawa wrote:
The RegisterDragDrop function requires OleInitialize to be called beforehand, and this check ensures that the resources initialized by OleInitialize are still allocated and have not been released. In the v2 patch, I added an InternalIsProcessInitialized call to explicitly verify that COM have been initialized by OleInitialize or CoInitializeEx. Thanks for reviewing. Could we check it in a more direct fashion, using COM_CurrentInfo()->ole_inits for example?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/10245#note_131809
On Wed Mar 11 10:35:49 2026 +0000, Nikolay Sivov wrote:
Could we check it in a more direct fashion, using COM_CurrentInfo()->ole_inits for example? That sounds much better. I'll submit the v3 patch soon. Thanks for the helpful tip.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/10245#note_131815
participants (3)
-
Akihiro Sagawa -
Akihiro Sagawa (＠sgwaki)
-
Nikolay Sivov (＠nsivov)