From 4874a7b13cad28ff2d1e6f30fbf9f2f55badf9bd Mon Sep 17 00:00:00 2001 From: Volker Hilsheimer Date: Thu, 20 Mar 2025 18:58:33 +0100 Subject: [PATCH] QComboBox: close the container explicitly before destroying it QComboBox destroys the container of the view in its own destructor, before the QWidget destructor is entered. The container is a toplevel widget, so destroying that will destroy a platform window, triggering an accessibility update cycle that calls back into Qt. As the QComboBox has not yet reached the QWidget destructor, it still considers itself as visible and focused. The accessibility query will therefore operate on it, as the focused object. Probing the state accesses the view. The view however is already partially destroyed, as the container's destruction has already passed the QWidget destructor deleting all its children. As a result, we are returning a pointer to a QAbstractItemView that's already in the QWidget destructor, resulting in a crash. Options for fixing this would be resetting the view pointer in ~QComboBoxPrivateContainer to nullptr and to test for that in client code. Doing that triggered crashes in tests, as QComboBox::view() so far has never returned a nullptr no matter the state of the combobox. So instead, close the container explicitly before destroying it. This way, any update cycle resulting in reentrancy (such as accessibility of backingstore flushing when the container closes) will be completed before objects are destroyed. This amends fde358dd9069d0695f113ec6ba98efebedd1e520, which added the explicit destruction of the container, for similar reasons. This seems to be a combobox-specific problem due to the combination of explicit destruction of (toplevel) child widgets, resulting event processing, and exposing of internal widget states through public API as part of the widget's accessible state. Pick-to: 6.8 6.5 Fixes: QTBUG-132310 Change-Id: I74df5b71906ce8153b12ddc35b897a44e7752907 Reviewed-by: Axel Spoerl (cherry picked from commit 74e5a51babe0a133ce0fe9b907ef77cc606fbcb9) Reviewed-by: Qt Cherry-pick Bot --- src/widgets/accessible/complexwidgets.cpp | 2 +- src/widgets/widgets/qcombobox.cpp | 10 ++++++++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/widgets/accessible/complexwidgets.cpp b/src/widgets/accessible/complexwidgets.cpp index 77bf8504fa3..9c09b9fc9ff 100644 --- a/src/widgets/accessible/complexwidgets.cpp +++ b/src/widgets/accessible/complexwidgets.cpp @@ -441,7 +441,7 @@ QAccessible::State QAccessibleComboBox::state() const if (QComboBox *cBox = comboBox()) { s.expandable = true; - s.expanded = isValid() && cBox->view()->isVisible(); + s.expanded = isValid() && cBox->view() && cBox->view()->isVisible(); s.editable = cBox->isEditable(); } return s; diff --git a/src/widgets/widgets/qcombobox.cpp b/src/widgets/widgets/qcombobox.cpp index 5899568b8a2..00c23856280 100644 --- a/src/widgets/widgets/qcombobox.cpp +++ b/src/widgets/widgets/qcombobox.cpp @@ -1472,8 +1472,14 @@ QComboBox::~QComboBox() ; // objects can't throw in destructor } - // Dispose of container before QComboBox goes away - delete d->container; + // Dispose of container before QComboBox goes away. Close explicitly so that + // update cycles back into the combobox (e.g. from accessibility when the + // active window changes) are completed first. + if (d->container) { + d->container->close(); + delete d->container; + d->container = nullptr; + } } /*!