wxFileDialog doesn't work if the application calls ::CoInitializeEx (Issue #23578)
jaquilio
Description
wxFileDialog doesn't show up if the application initializes Com by calling ::CoInitializeEx.
This was working fine in earlier versions of wxWidgets at least up to 3.1.4 and isn't working in the current IFileDialog dialog based implementation present in wxWidgets version 3.2.2.1.
To reproduce
Use the dialogs sample included in wxWidgets distribution and modify it to call ::CoInitializeEx(NULL, COINIT_MULTITHREADED) at the beginning of the application.
Currently, the constructor of MyApp class in dialogs.h file of that sample looks like this:
MyApp() { m_startupProgressStyle = -1; }
Edit it to be the following:
MyApp()
{
HRESULT hr = ::CoInitializeEx(NULL, COINIT_MULTITHREADED);
wxASSERT(!FAILED(hr));
m_startupProgressStyle = -1;
}
After making the above edit, build the dialogs sample and run it. From the menu, select Dialogs->File operations->Open File. Note that the file dialog doesn't appear.
Platform and version information
- wxWidgets version 3.2.2.1
- wxWidgets port: wxMSW
- Windows 11, built using the latest (17.6.2) version of Microsoft Visual Studio 2022 Community Edition
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
PB
I forgot the little I knew about COM long time ago, so I have no idea whether calling ::CoInitializeEx() with COINIT_MULTITHREADED from the main thread is correct.
I will just add what the OP forgot to mention: The code (GUI thread?) gets stuck in IFileDialog::Show() call here:
https://github.com/wxWidgets/wxWidgets/blob/f2f2868de55901c0b99d1e4a804063767f258c0d/src/msw/dirdlg.cpp#L295
When I use "Break All" from the MSVS Debug menu, the threads window contains this:
Not Flagged > 18904 0 Main Thread Main Thread combase.dll!MTAThreadWaitForCall
Not Flagged 12072 0 Worker Thread combase.dll!CRpcThreadCache::RpcWorkerThreadEntry combase.dll!WaitCoalesced
Not Flagged 8340 0 Worker Thread ntdll.dll!TppWorkerThread ntdll.dll!NtWaitForWorkViaWorkerFactory
Not Flagged 20056 0 Worker Thread ntdll.dll!TppWorkerThread ntdll.dll!NtWaitForWorkViaWorkerFactory
Not Flagged 12628 0 Worker Thread combase.dll!CRpcThreadCache::RpcWorkerThreadEntry win32u.dll!NtUserCallHwndParamLockSafe
Not Flagged 17536 0 Worker Thread msvcrt.dll!_threadstartex win32u.dll!NtUserMsgWaitForMultipleObjectsEx
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
jaquilio
Regarding whether it is correct to call ::CoInitializeEx, the following comments in include/wx/ole/oleutils.h seem relevant:
Don't consider it to be an error as
// we don't actually care ourselves about the mode used so this allows the
// main application to call OleInitialize() on its own before we do if it
// needs non-default mode.
OK, it is talking about OleInitialize there and not CoInitialize, but, looks like the idea is to allow the main application to call.
In our case, we ended up having to call ::CoInitializeEx to use services such as WMI, Windows Audio etc.
Also, as mentioned in the forum thread, this has been working fine for years with earlier versions of wxWidgets.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
PB
FWIW, I have also noticed that there is a difference in whether the file dialog has a parent (owner) or not: It works without the parent:
#include <wx/wx.h> #include <wx/filedlg.h> class MyApp : public wxApp { public:
MyApp()
{
HRESULT hr = ::CoInitializeEx(NULL, COINIT_MULTITHREADED);
wxASSERT(!FAILED(hr));
}
virtual bool OnInit() override
{
wxFrame* frame = new wxFrame(nullptr, wxID_ANY, "Test");
frame->Show();
int flags = wxFD_SAVE | wxFD_OVERWRITE_PROMPT;
#if 1 // changing 1 to 0 results to fileDialog not shown if ::CoInitializeEx() called with COINIT_MULTITHREADED
flags |= wxDIALOG_NO_PARENT;
#endif
wxFileDialog fileDialog(nullptr, "Save text file", "", "", "Text files (*.txt)|*.txt", flags);
fileDialog.ShowModal();
return true;
}
}; wxIMPLEMENT_APP(MyApp);;I have no idea what makes the difference, it may also be a red herring.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
I didn't have time to test it yet, but I'm afraid that this means that using IFileDialog is simply not supported with COINIT_MULTITHREADED, i.e. you have to use COINIT_APARTMENTTHREADED for the main/UI thread in order to use it.
We could handle this more gracefully by falling back to the old file dialog functions if we detect that the unsupported COM mode was used, but this is still going to be not ideal.
Is there any particular reason you can't use COINIT_APARTMENTTHREADED in your application?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
jaquilio
COM documentation says the following:
If you specify apartment threading, you are making the following guarantees:
- You will access each COM object from a single thread; you will not share COM interface pointers between multiple threads.
- The thread will have a message loop.
If either of these constraints is not true, use the multithreaded model.
Those two constraints are quite serious restrictions and satisfying those will in general be hard, including in our application.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
PB
FWIW, I can reproduce the issue with pure minimal Win32 code, so it appears that vadz was right and this is not a wxWidgets issue. When IFileDialog has an owner and COINIT_MULTITHREADED is used, the code gets stuck:
#include <cassert> #include <shobjidl_core.h> #include <windows.h> LRESULT CALLBACK WndProc(HWND hWnd, UINT message, WPARAM wParam, LPARAM lParam) { if ( message == WM_DESTROY ) { PostQuitMessage(0); return 0; } return DefWindowProc(hWnd, message, wParam, lParam); } int WINAPI WinMain(HINSTANCE, HINSTANCE, LPSTR, int nCmdShow) { HRESULT hr = ::CoInitializeEx(NULL, COINIT_MULTITHREADED); assert(!FAILED(hr)); LPCWSTR className = L"TestFrame"; WNDCLASSEX wcex = {0}; wcex.cbSize = sizeof(WNDCLASSEX); wcex.lpfnWndProc = WndProc; wcex.hCursor = LoadCursor(nullptr, IDC_ARROW); wcex.hbrBackground = (HBRUSH)(COLOR_WINDOW+1); wcex.lpszClassName = className; RegisterClassExW(&wcex); HWND hFrame = CreateWindowExW(0, className, L"Test Frame", WS_OVERLAPPEDWINDOW, CW_USEDEFAULT, CW_USEDEFAULT, 400, 400, nullptr, nullptr, nullptr, nullptr); ShowWindow(hFrame, nCmdShow); UpdateWindow(hFrame); IFileDialog *pfd; if ( SUCCEEDED(CoCreateInstance(CLSID_FileOpenDialog, NULL, CLSCTX_INPROC_SERVER, IID_PPV_ARGS(&pfd))) ) { pfd->SetOptions(FOS_FORCEFILESYSTEM); #if 1 // change 1 to 0 make pfd appear pfd->Show(hFrame); #else pfd->Show(nullptr); #endif pfd->Release(); } MSG msg; while ( GetMessage(&msg, nullptr, 0, 0) ) { TranslateMessage(&msg); DispatchMessage(&msg); } return (int)msg.wParam; }
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
oneeyeman1
@PBfordev ,
Probably make sense to file a bug with MS?
Thank you.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
I just took a look at the COM documentation. It says the following:
If you specify apartment threading, you are making the following guarantees: - You will access each COM object from a single thread; you will not share COM interface pointers between multiple threads. - The thread will have a message loop. If either of these constraints is not true, use the multithreaded model.
It seems to me that those two constraints are quite serious restrictions and satisfying those in general will be hard, including in our application.
The second one is always true, by definition, for the UI thread of a wxWidgets application, so it's not a constraint at all. The first one does mean that you can't share COM pointers between the UI and other threads, but are you sure you need to do it?
In any case, I have trouble seeing what else can we do here than what I proposed in the previous comment, and I'll try to implement this, but I think you should still seriously consider not using COINIT_MULTITHREADED in your application because you won't be able to use the new style file dialogs if you keep using it.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
BTW, MSDN says:
Note If COM is initialized using CoInitializeEx with the COINIT_MULTITHREADED flag, SHBrowseForFolder fails if the calling application uses the BIF_USENEWUI or BIF_NEWDIALOGSTYLE flag in the BROWSEINFO structure.
so even the old style functions don't work with COINIT_MULTITHREADED. It really looks like you should avoid using it for the main thread.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
Please check the linked PR with your application -- it should fix the problem, at the cost of using old style dialogs.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
jaquilio
Thanks for your comments @vadz.
Yesterday, I switched our application to use APRTMENTTHREADED instead of MULTITHREADED and to the extent tested so far, things are working fine and nothing is broken. Keeping fingers crossed :).
so even the old style functions don't work with COINIT_MULTITHREADED.
May be I misunderstand what you mean old style functions. Our application has been using COINIT_MULTITHREADED for several years and things have been working fine. Anyway, hopefully, it is a moot point for us now since APRTMENTTHREADED seems to be working fine.
Thanks.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
Closed #23578 as completed via 6c4bee8.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Hajo Kirchhoff
hajokirchhoff left a comment (wxWidgets/wxWidgets#23578)Beginning with Windows 11 24H2 this fix no longer seems to work and the problem is back.
Note that our application works fine with pre 24H2, but hangs as soon as we open a file dialog for the second time in 24H2. Yes, for the second time. The first open works okay, the second open does not.
We debugged wxFileDialog::ShowModal() and we are seeing the following behaviour:
- we initialize COM with
CoInitializeEx(0, COINIT_APARTMENTTHREADED);in our main thread at startup (OnInit) - when we open the file dialog for the first time, the attempt to change the threading model with CoInitializeEx in line 1280 fails
const HRESULT hr = ::CoInitializeEx(NULL, COINIT_APARTMENTTHREADED); // returns RPC_E_CHANGED_MODE
and the old method is used ShowCommFileDialog.
We open the file dialog again, but this time (the second time) const HRESULT hr = ::CoInitializeEx(NULL, COINIT_APARTMENTTHREADED); returns S_OK, canUseIFileDialog returns true, ShowIFileDialog is called and hangs.
I have no idea who or what is changing the threading model in the mean time.
This is the code in the "OnButtonClick" method that shows two file dialogs right after another.
void hidden_mainwindow::OnButtonClick(wxCommandEvent& event) { { auto filedlg = wxFileDialog(this, _("Select a file to open"), wxEmptyString, wxEmptyString, wxT("All files (*.*)|*.*"), wxFD_OPEN | wxFD_FILE_MUST_EXIST | wxFD_CHANGE_DIR); filedlg.ShowModal(); } { auto filedlg = wxFileDialog(this, _("Select a file to open"), wxEmptyString, wxEmptyString, wxT("All files (*.*)|*.*"), wxFD_OPEN | wxFD_FILE_MUST_EXIST | wxFD_CHANGE_DIR); filedlg.ShowModal(); } }
As you can see, absolutely no processing is done in the mean time. It's just "open" then another "open".
I also wonder why the first attempt returns an "RPC_E_CHANGED_MODE" at all, since we explicitly used APARTMENTTHREADED in our own CoInitializeEx.
Any idea what else we could do?
tnx
Hajo
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#23578)It looks like RPC_E_CHANGED_MODE is only returned once and not every time, so we need to either remember that it did or, maybe, use CoGetApartmentType() instead.
This would prevent hanging, but still won't allow you to use the new style dialogs which is what the users typically want, so I'd still recommend finding why exactly does the COM initialization mode changes: someone must be calling CoInitializeEx() with a different mode, so putting a breakpoint on it should allow you to find out who does it.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#23578)The simplest workaround I can see is this:
diff --git a/src/msw/dirdlg.cpp b/src/msw/dirdlg.cpp index a2b9ee9e87..043bd72897 100644 --- a/src/msw/dirdlg.cpp +++ b/src/msw/dirdlg.cpp @@ -152,9 +152,12 @@ int wxDirDialog::ShowModal() // Use IFileDialog under new enough Windows, it's more user-friendly
Hajo Kirchhoff
hajokirchhoff left a comment (wxWidgets/wxWidgets#23578)It might work, but the cause for this is kind of weird. First, some more background:
- Our app uses the Threading Apartment Model, because we register an ATL Server with _ATL_FREE_THREADING.
- That is why in the ShowModal function the first call to CoInitializeEx returns RPC_E_CHANGED_MODE and the Dialog uses the old File Dialog (which is fine by us).
But now it get's completely weird, at least on Win11 24H2.
After the call to ::GetOpenFileName(W) in DoShowCommFileDialog happened and returned, because I closed the dialog with CANCEL, it looks as if I can suddenly change the threading model !
auto filedlg = wxFileDialog(this, _("Select a file to open"), wxEmptyString, wxEmptyString, wxT("All files (*.*)|*.*"), wxFD_OPEN | wxFD_FILE_MUST_EXIST | wxFD_CHANGE_DIR);
// (1) hr = CoGetApartmentType(&aptType, &aptQualifier); filedlg.ShowModal(); // (2) hr = CoGetApartmentType(&aptType, &aptQualifier); hr = ::CoInitializeEx(NULL, COINIT_APARTMENTTHREADED); // (3) hr = CoGetApartmentType(&aptType, &aptQualifier);
I queried the apartment type at three different points.
- right before the call to ShowModal/DoShowCommFileDialog
where it still is MTA (multithreaded apartment) - immediately after the call
where it is still MTA, but with the qualifier APTTYPEQUALIFIER_IMPLICIT_MTA
and finally - after another call to CoInitializeEx
where it suddenly changes the type to APTTYPE_MAINSTA (qualifier none)
So it looks to me as if the call to ::GetOpenFileName actually/accidentially(?) resets the threading model of the thread.
So the first call to CoInitializeEx afterwards will set a new application threading model, which will be set to APARTMENTTHREADED which might however be bogus, since IFileDialog will still hang.
We'll test this behaviour on Windows 10 later this day.
So your suggestion for testing this only once might work, however the apartment model might suddenly, unexpectedly change.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#23578)I have no idea what's going on here, but I wonder if this is reproducible on all systems or just some particular one? In the latter case it could be due to some rogue shell extension...
Anyhow, I think we should still commit the patch above, do you agree?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Hajo Kirchhoff
hajokirchhoff left a comment (wxWidgets/wxWidgets#23578)I have no idea what's going on here, but I wonder if this is reproducible on all systems or just some particular one?
It's reproducible on two entirely different Win11 24H2 systems. One is my workstation with lots of shell extensions, the other is on a machine that will be shipped to a customer, an almost clean, fresh install of Win11 24H2. So I guess it's not a shell extension.
Anyhow, I think we should still commit the patch above, do you agree?
Yes. The patch makes sense. And perhaps for future, similar problems, a way to disable the IFileDialog programmatially. Wasn't there a "Feature Flag" mechanism in wxWidgets? Something like "wxSetFeature(...)"?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#23578)OK, I'll push this soon, thanks.
There is wxSystemOption but it's really a hack and I'd prefer to avoid using it unless absolutely necessary.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.