Details
-
Bug
-
Resolution: Fixed
-
P1: Critical
-
6.5.3, 6.6.3, 6.7.3, 6.7, 6.8.0
-
None
-
Originaly reproduced on Arch Linux, but it seems reproducible at least on Ubuntu too. With various version of Qt6.
-
665e1ea7a (dev), ec16b18d8 (6.8), 5b04408cf (tqtc/lts-6.5)
Description
This is detailed description and technical deep-dive of issue https://github.com/FreeCAD/FreeCAD/issues/16616
Summary:
The mentioned application (FreeCad) crashed under specific circumstances when closing tab in tabBar for MDI window. The easiest way to reproduce this is to open 2 MDI windows and click on close icon of the inactive tab in tabBar. Few more specific conditions need to be met to reproduce this, which will be described latter.
Issue identification
The SIGSEG was originally pointed to:
qmdiarea.cpp:1372
QMdiAreaPrivate::subWindowList() {
...
childWindows.at(indicesToActivatedChildren.at( i))
Further investigation showed the childWindows have already only one item (the closed window was already removed), the indicesToActivatedChildren have also one item (expected), but the item contains 1, which is invalid index to childWindows. The resulting call of {{childWindows.at(1) of course }}returns garabage.
Further investigation proved the FreeCad, where the bug manifests, have custom handling on events related to MDI TabBar and MDI windows, which is relevant as pointed latter.
The investigation further led to function QMdiArea::viewportEvent() where QEvent::ChildRemoved event is handled. The following lines of the code are involved
{{qmdiarea.cpp:2426 }}
QMdiArea::viewportEvent(QEvent *event) {
...
d->childWindows.removeAt( i){};
d->indicesToActivatedChildren.removeAll( i){};
d->updateActiveWindow(i, activeRemoved);{}
The first two lines of code are capable of creating the situation which leads to the SIGSEG, but the call to updateActiveWindow contains code which "fixes" the content of indicesToActivatedChildren. Unfortunately the fix for updateActiveWindow is applied only after tabBar update. This it-self would not cause any trouble, but the remove of the tab can trigger event, which is in this case handled by the application. The event handler does few more calls, one of which leads to the use of, now invalid, content of indicesToActivatedChildren.
qmdiarea.cpp:1077
void QMdiAreaPrivate::updateActiveWindow() {
...
if (tabBar && removedIndex >= 0) {
const QSignalBlocker blocker(tabBar);
tabBar->removeTab(removedIndex);
updateTabBarGeometry();
{
{ }}}
...
// The event is triggered right here and application handling the
// event works does call when indicesToActivatedChildren{{ is not valid yet, the code bellow is needed to fix it.}}
...
for (int i = 0; i < indicesToActivatedChildren.size(); ++i) {
int *index = &indicesToActivatedChildren[i];
if (*index > removedIndex)
--*index;
{{ }
}}
The attached patch is proposed and was tested as a fix.