`QGuiApplication::focusWindow()` can point to deallocated memory causing read access violations.

AnyOldName3

For context, I'm working on OpenMW's Construction Set (source code available here: https://gitlab.com/OpenMW/openmw/tree/master/apps/opencs). I've written nearly none of it, and so can't guarantee that it's not doing anything wrong, so I'm asking this as a question rather than reporting it as a bug. Besides normal Qt stuff, the application uses osgQt for some 3D rendering. While the application is cross-platform and should be backwards compatible with Qt 4, I'm using Visual Studio 2017 on Windows, and Qt 5.12.2.

While investigating an unrelated issue, I noticed that sometimes, when exiting the application (compiled in Debug mode) it was reporting a read access violation when QWidget::isActiveWindow() tried to cast QGuiApplication::focusWindow() to a QWidgetWindow after the window in question had already been deleted.

The QWindow destructor calls QWindowPrivate::destroy(), which sets the focus window to something else (as long as the platformWindow isn't null), so theoretically, the focus window pointer shouldn't point to deallocated memory.

However, this isn't enough. When the QWindowsWindow is destroyed, it calls its destroyWindow method, which has a call DestroyWindow(m_data.hwnd). This causes Windows to generate a refocus event, and then Qt obeys that, even though the requested window has already been destroyed, so has a null platformWindow. When the window later gets deleted, QWindowPrivate::destroy() exits early because the platformWindow is already null, so the focus window isn't set back to one that actually exists (or a nullptr) and deallocated memory can get read if the pointer is followed.

The issue happens reliably if I do certain things, and reliably doesn't happen if I do other things, and adding breakpoints can cause the error to go away (by switching focus to Visual Studio, so the focus window pointer becomes null), so it's been a bit of a pain to investigate this.

Could this conceivably be that the application is misusing Qt, or is this a Qt bug that's somehow not been reported before?

SGaist

Hi and welcome to devnet,

Technically, it could be both but since you can trigger that pretty easily, it could very well be that you found a bug. Can you reproduce the issu with a minimal compilable example ?

AnyOldName3

It's quite a big application, so I'm not confident that I'd be able to replicate whatever the specific combination and order of actions that triggers the bug in a new application without first knowing what the problem was (and if I knew what the problem was, I could fix it). I think it might be timing-sensitive, too, as at one point, adding a line to an unrelated destructor was switching between having the issue and not. I could provide a log of what the application spits out when it's full of tracepoints in interesting locations if that's wanted, which I imagine might help.

kshegunov

@AnyOldName3 said in `QGuiApplication::focusWindow()` can point to deallocated memory causing read access violations.:

I could provide a log of what the application spits out when it's full of tracepoints in interesting locations if that's wanted, which I imagine might help.

This would be nice, also a backtrace is always useful.

AnyOldName3

Here's the log: https://gitlab.com/snippets/1846991

The XML tracepoint descriptions are here: https://gitlab.com/snippets/1846986

I'll describe the tracepoints in a more human-readable way, too. Some of them include the call stack, but not line numbers for some reason known only to Microsoft.

qguiapplication.cpp line 2282: Says when QGuiApplicationPrivate::processActivatedEvent is run, including the event pointer, the previous focus and the new focus.
qwindow.cpp line 1863: Says when QWindowPrivate::destroy exits early as the platform window is null.
qwindowswindow.cpp lines 1300 and 1301: Says when these lines are run to demonstrate that it's the DestroyWindow(m_data.hwnd) creating the refocus events.
qwindowsysteminterface.cpp line 83: Says when a window system event is received of type ActivatedWindow, and includes the event pointer (so you can pair these up with when they're processed later on).
Data breakpoint on 0x00007FFF612CAC10: This address is where QGuiApplicationPrivate::focus_window resides, but annoyingly it's displayed as an address rather than a variable name. This tracepoint says when the focus window changes.

The actual crash occurs with this backtrace, although it's not especially interesting and is described in the first post - QWidget::isActiveWindow() tried to cast QGuiApplication::focusWindow() to a QWidgetWindow after the window in question had already been deleted.

kshegunov

I looked a bit at the code and at the trace. I couldn't spot anything out of the ordinary. However it seemed to me very unlikely that this refocus event you talk about is an issue in that particular sequence, as the refocusing doesn't actually spin the event loop; I'm just not quite convinced (doesn't mean necessarily it isn't a bug).
An odd issue, indeed.

AnyOldName3

So you're thinking that maybe the problem is that the event loop is being run after things have started closing and it's not supposed to do that?

Anyhow, in case it helps, I made an equivalent trace but without the issue. I did this by clicking into another application's window and back before closing my application.

https://gitlab.com/snippets/1848606

Hopefully, there's some critical difference between the two that makes everything clear.

EDIT: I ran both through a diff tool, and the interesting differences seemed to start around line 1340.

SGaist

The snippet is not accessible.

AnyOldName3

Oops... It should be fixed now.

kshegunov

I looked again. I don't know. My best guess is that some UI object (i.e. QWindow/QWidget) outlives the application object.
But to be completely blunt, I had hard time getting my bearings straight, that code is some mess - STL, boost, Qt and more amalgamated together in something very hard to read.

AnyOldName3

I've checked, and the problem occurs while the QApplication is still around. There's even a check in the QApplication destructor that tells you if there are still widgets alive, so I'd have noticed if that was the issue. Do you have any other ideas?

SGaist

Well... The technique is not the fastest but one thing you can do is comment everything out to start with an empty main window and then gradually add back pieces of your application until it breaks. That should help point which part might be responsible.

AnyOldName3

It's very timing-sensitive, so even subtly changing parts of the code that are completely unrelated can make it go away. That means I can't get a reliable negative test, so can't tell if adding something back caused the bug or just triggered it to reveal itself again.

mr.bachmann

I ran into the same problem and the issue occurs when closing the last Window (e.g. a dialog) and the dialog is destroyed. When creating a new dialog or widget the next time, the pointer returned by QGuiApplication::focusWindow() is a dangling pointer since it points to an already destroyed object.
The following code works around the issue and triggers an update of the pointer returned by QGuiApplication::focusWindow() after having closed the last Window by sending a FocusOut event:

QObject::connect(qobject_cast<QGuiApplication*>(QGuiApplication::instance()), &QGuiApplication::lastWindowClosed, []()
{
   QEvent event(QEvent::FocusOut);
   QCoreApplication::instance()->sendEvent(QCoreApplication::instance(), &event);
   QCoreApplication::instance()->processEvents();
});

The Qt version which I used was 5.11.1.

kshegunov

@mr-bachmann said in `QGuiApplication::focusWindow()` can point to deallocated memory causing read access violations.:

I ran into the same problem and the issue occurs when closing the last Window (e.g. a dialog) and the dialog is destroyed. When creating a new dialog or widget the next time, the pointer returned by QGuiApplication::focusWindow() is a dangling pointer since it points to an already destroyed object.

Thanks for investigating. Do you have a minimal example that reproduces this? (I can see in the Qt code where the problem lies:
https://code.woboq.org/qt5/qtbase/src/gui/kernel/qguiapplication.cpp.html#_ZN22QGuiApplicationPrivate20emitLastWindowClosedEv should clear the global variable or send the FocusOut, but it'd be good to file this as a bug report).