Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Qt freezes when displaying native dialogs with briefcase>=0.3.9 #930

Closed
davidfokkema opened this issue Oct 19, 2022 · 8 comments · Fixed by beeware/briefcase-windows-VisualStudio-template#11
Labels
bug A crash or error in behavior. windows The issue relates to Microsoft Windows support.

Comments

@davidfokkema
Copy link
Contributor

When using Qt as the GUI toolkit, my app freezes when displaying a native dialog. For example, this code freezes my app:

dialog = QtWidgets.QFileDialog()
dialog.exec()

This only happens when executing briefcase run; running briefcase dev results in the expected behaviour. Also, it only does not work while executing in the main event loop. During UI class instantiation, it is no problem to display the dialog. Also, when you explicitly ask for a non-native dialog, QFileDialog() works fine. Older briefcases work without problems so I suspect it has something to do with the new stub app approach.

Steps to reproduce the behavior:

  1. Start a new briefcase project with all defaults but select either PySide2 or PySide6 as the GUI toolkit.
  2. Change the main app.py class as follows:
class HelloWorld(QtWidgets.QMainWindow):
    def __init__(self):
        super().__init__()
        self.init_ui()
        
        # Showing dialogs before entering the main Qt event loop works
        #dialog = QtWidgets.QFileDialog()
        #dialog.exec()
        
        # Wait to display dialog during main event loop
        QtCore.QTimer.singleShot(1000, self.open_dialog)

    def init_ui(self):
        self.setWindowTitle('helloworld')
        self.show()

    def open_dialog(self):
        # Non-native file dialog does work
        # dialog = QtWidgets.QFileDialog(options=QtWidgets.QFileDialog.DontUseNativeDialog)
        dialog = QtWidgets.QFileDialog()
        dialog.exec()

Expected behavior
I expect the native file dialog to be displayed without problems.

Environment:

  • Operating System: Windows 3.10 (mainly tested in Parallels VM, sorry)
  • Python version: 3.10
  • Software versions:
    • Briefcase: 0.3.9, 0.3.10, 0.3.11 (tested all three)
    • Qt: Qt5, Qt6 with PySide2 and PySide6, tested on many minor versions.
@davidfokkema davidfokkema added the bug A crash or error in behavior. label Oct 19, 2022
@freakboy3742
Copy link
Member

Thanks for the report. It certainly seems to point to a problem with the stub app; if the app generally works, but dialogs don't, it suggests to me that the threading mode on the main executable hasn't been set correctly.

I'm at a conference right now so I don't have access to my Windows box for testing; but it's possible this might affect Toga dialogs as well.

@freakboy3742 freakboy3742 added up-for-grabs windows The issue relates to Microsoft Windows support. labels Oct 19, 2022
@davidfokkema
Copy link
Contributor Author

Enjoy the conference!

I just tested with briefcase new and accepting all defaults, thus selecting Toga as the GUI toolkit. The default app seems to work fine. Selecting Help -> About helloworld from the dialog opens a new dialog, seemingly without problems. I'm not sure how significant that is, but it seems possible this only affects Qt.

@davidfokkema
Copy link
Contributor Author

davidfokkema commented Feb 14, 2023

I've taken this as far as I can go.

TLDR: can you please include CoUninitialize() to the top of Main() or do you expect problems?

Long story:
I installed cookiecutter and briefcase and used cookiecutter on the https://github.com/beeware/briefcase-windows-VisualStudio-template repository. I needed some time to figure out how to kickstart my app and install PySide6, but I got it to work, which resulted in the same error as above: the system hangs when displaying native dialogs. Good! I also found another reference to the problem: https://wiert.me/2016/02/03/windows-vista78-hangs-for-windows-common-dialogs-when-your-com-initialisation-is-wrong/.

It took some time to figure out the console thing which wasn't working from the terminal. I needed some extra code. For reference:

    HANDLE stdHandle;
    int hConsole;
    FILE* fp;

    AttachConsole(ATTACH_PARENT_PROCESS);
    stdHandle = GetStdHandle(STD_OUTPUT_HANDLE);
    hConsole = _open_osfhandle( (long)stdHandle, _O_TEXT);
    fp = _fdopen(hConsole, "w");
    _wfreopen_s( &fp, L"CONOUT$", L"w", stdout);
    _wfreopen_s( &fp, L"CONOUT$", L"w", stderr);

Anyway, the following simple fix (should've trusted StackOverflow sooner) works: just add this line to the top of Main()

CoUninitialize();

This will uninitialize the thread model. I have confirmed, by calling CoInitializeEx() and checking the return value, that the thread model has already been initialised before Main() is called. I can't figure out why. I stripped down Main.cpp to:

#include <cstdio>
#include <io.h>

#include <fcntl.h>
#include <windows.h>

int Main() {
    HANDLE stdHandle;
    int hConsole;
    FILE* fp;
    HRESULT hr;

    // CoUninitialize();

    AttachConsole(ATTACH_PARENT_PROCESS);
    stdHandle = GetStdHandle(STD_OUTPUT_HANDLE);
    hConsole = _open_osfhandle( (long)stdHandle, _O_TEXT);
    fp = _fdopen(hConsole, "w");
    _wfreopen_s( &fp, L"CONOUT$", L"w", stdout);
    _wfreopen_s( &fp, L"CONOUT$", L"w", stderr);

    hr = CoInitializeEx(NULL, COINIT_APARTMENTTHREADED);
    if (hr == S_OK) printf("Was not yet called\n");
    if (hr == S_FALSE) printf("Was already called\n");
    if (hr == RPC_E_CHANGED_MODE) printf("Different mode\n");

    return 0;
}

and tried to slim down the .vcxproj file to not include references to CrashDialog.cpp, Reference Includes to System, and what not. No matter, it keeps saying that CoInitializeEx() was already called. I'm not used to Visual Studio and not used to C++ so I might have missed something. All Microsoft docs tell you that you really need to call CoInitializeEx() yourself. Not so, in this case. @freakboy3742 previously mentioned that "the threading mode on the main executable hasn't been set correctly" which may mean that I missed a setting somewhere? Anyway, hopefully this will help 😉 .

@davidfokkema
Copy link
Contributor Author

davidfokkema commented Feb 14, 2023

Just to be sure, I checked if a simple 1 / 0 in my app would still throw up the briefcase crash dialog with traceback. Was a little bit worried about letting briefcase call Windows APIs to generatie a window without the previous call to CoInitializeEx(). That still works after CoUninitialize() has been included.

Mind that in order to use CoUninitialize() the Visual Studio project settings must be adjusted. In properties -> Linker -> Input -> Additional Dependencies (which is empty), Ole32.lib must be included.

@freakboy3742
Copy link
Member

This is awesome debugging, @davidfokkema - thanks! AFAICT, this is exactly what is needed; adding this extra line doesn't disrupt anything with the crash dialog or Toga's own behavior, and I can confirm it also fixes the issue for me on Qt - so I'll call that a significant win!

I've just opened beeware/briefcase-windows-VisualStudio-template#11 to incorporate this change; you should be able to test that yourself by adding template_branch = 'threading-mode' to your pyproject.toml, and rebuilding the app in VisualStudio mode. If you can confirm this works, then I'll merge the PR, retag the app repo so the stub binaries, and back port to 0.3.12.

@davidfokkema
Copy link
Contributor Author

Works like a charm! 😃

@freakboy3742
Copy link
Member

@davidfokkema I've just merged the VisualStudio template change, and rebuilt the stub apps on the app template. The patches have also been back ported to the v0.3.12 template branches. Let me know if you have any problems.

@davidfokkema
Copy link
Contributor Author

My app builds and works perfectly now on briefcase 0.3.12, freezes after a downgrade to 0.3.11 and works again after upgrading to 0.3.12. I tested that cycle just to make sure I had no modifications floating around and that stock briefcase 0.3.12 fixes the issue. It does, so I’m very happy. Thanks!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A crash or error in behavior. windows The issue relates to Microsoft Windows support.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants