slint-ui / slint

Slint is a declarative GUI toolkit to build native user interfaces for Rust, C++, or JavaScript apps.
https://slint.dev
Other
16.94k stars 568 forks source link

Qt backend: Application crashes when closing a `PopupWindow` with a right click #4129

Closed nununoisy closed 9 months ago

nununoisy commented 9 months ago

Steps to reproduce:

Expected outcome: The PopupWindow should close, matching the behavior of left- or middle-clicking on the application window.

Actual outcome: The application crashes with a return of 0xc0000005 (STATUS_ACCESS_VIOLATION).

You can try reproducing the bug in GBPresenter v0.6.0 - an affected PopupWindow can be opened by clicking on the info button in the top right corner. I can also reproduce the bug within slint-viewer when viewing main.slint, so I'm confident that the bug is in Slint.

I am using:

ogoffart commented 9 months ago

Thanks a lot for the bug report.

I couldn't reproduce the issue on Linux. And I don't have a Qt setup on my windows VM at the moment. Do you have a backtrace by any chance? As I workaround, i recommend not using the Qt backend on windows.

(Opening the main.slint from gbpresenter with the slint-viewer, i noticed another panic fixed in https://github.com/slint-ui/slint/pull/4140)

tronical commented 9 months ago

I've tried to reproduce this but I can't seem to make it crash. This is with current slint, Qt 6.5.1 (MSVC). Here's a clip. Am I doing the wrong steps somehow?

https://github.com/slint-ui/slint/assets/1486/6c9d4e74-cc02-4bf2-9da1-7ab94f119c80

nununoisy commented 9 months ago

I did a bit of debugging and I think I figured out what's going on:

While forwarding the QContextMenuEvent, I noticed that the active popup widget pointer was nullptr, which explains why the program segfaulted. I think the problem here is that the SlintWidget is sometimes deleted before all mouse events are delivered. The docs for QObject::~QObject() state that a crash may occur if an object is deleted while there are undelivered pending events waiting for it. Although the mouse event handler attempts to check to see if the popup closes to prevent a crash like this, the check doesn't work as Slint always eats window close events so it can forward them to downstream code without blocking. The fact that this is potentially a race condition might also explain why it isn't consistent.

A potential solution would be to have QtWindow hold a raw SlintWidget pointer instead of a smart pointer, and add a custom Drop impl that:

ogoffart commented 9 months ago

Thanks for the detailed debug! This is very useful. This is strange that we can't reproduce it for us. But you might be right that we should use deleteLater to destroy. We can probably do something like that: https://stackoverflow.com/a/41408511