Uploaded image for project: 'Qt'
  1. Qt
  2. QTBUG-129596

Crash when closing inactive tab in tabbar for MDI window

    XMLWordPrintable

Details

    • Bug
    • Resolution: Fixed
    • P1: Critical
    • 6.5.8, 6.8.1, 6.9.0 FF
    • 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.

      Attachments

        No reviews matched the request. Check your Options in the drop-down menu of this sections header.

        Activity

          People

            vhilshei Volker Hilsheimer
            pinky Jiří Pinkava
            Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved:

              Gerrit Reviews