From c6d6e05aaeb76d19b0cc921d1a0cb61ca8ca7677 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B8ger=20Hanseg=C3=A5rd?= Date: Mon, 24 Jun 2024 17:15:23 +0200 Subject: [PATCH] Fix leak of QWindowsUiaMainProvider instances Pair up all calls to QWindowsUiaMainProvider::providerForAccessible with a corresponding QWindowsUiaMainProvider::Release(). This fixes memory leaks when the Narrator application is running for applications that are recreating UI elements frequently. RAII ComPtr is not used here because going directly to ComPtr would make the change harder to review. Switching to ComPtr can be done in a separate patch. When the Narrator application is running, we may see that the memory usage temporarily increases, but it is reclaimed later. Fixes: QTBUG-126530 Change-Id: I1fd76da5759354633dbf040ba42a007d349264a6 Reviewed-by: Oliver Wolff (cherry picked from commit 0e65cbc82fbd8585a201c7feb16df410078a8cfb) Reviewed-by: Qt Cherry-pick Bot --- .../uiautomation/qwindowsuiaaccessibility.cpp | 1 + .../qwindowsuiagriditemprovider.cpp | 2 +- .../uiautomation/qwindowsuiagridprovider.cpp | 2 +- .../uiautomation/qwindowsuiamainprovider.cpp | 25 ++++++++++++++----- .../qwindowsuiaselectionitemprovider.cpp | 4 +-- .../qwindowsuiaselectionprovider.cpp | 4 +-- .../qwindowsuiatextrangeprovider.cpp | 2 +- 7 files changed, 27 insertions(+), 13 deletions(-) diff --git a/src/plugins/platforms/windows/uiautomation/qwindowsuiaaccessibility.cpp b/src/plugins/platforms/windows/uiautomation/qwindowsuiaaccessibility.cpp index 58924932818..3385959cc4e 100644 --- a/src/plugins/platforms/windows/uiautomation/qwindowsuiaaccessibility.cpp +++ b/src/plugins/platforms/windows/uiautomation/qwindowsuiaaccessibility.cpp @@ -48,6 +48,7 @@ bool QWindowsUiaAccessibility::handleWmGetObject(HWND hwnd, WPARAM wParam, LPARA if (QAccessibleInterface *accessible = window->accessibleRoot()) { QWindowsUiaMainProvider *provider = QWindowsUiaMainProvider::providerForAccessible(accessible); *lResult = UiaReturnRawElementProvider(hwnd, wParam, lParam, provider); + provider->Release(); return true; } } diff --git a/src/plugins/platforms/windows/uiautomation/qwindowsuiagriditemprovider.cpp b/src/plugins/platforms/windows/uiautomation/qwindowsuiagriditemprovider.cpp index a76128df584..8832c015d55 100644 --- a/src/plugins/platforms/windows/uiautomation/qwindowsuiagriditemprovider.cpp +++ b/src/plugins/platforms/windows/uiautomation/qwindowsuiagriditemprovider.cpp @@ -129,7 +129,7 @@ HRESULT STDMETHODCALLTYPE QWindowsUiaGridItemProvider::get_ContainingGrid(IRawEl return UIA_E_ELEMENTNOTAVAILABLE; if (QAccessibleInterface *table = tableCellInterface->table()) { - *pRetVal = QWindowsUiaMainProvider::providerForAccessible(table); + *pRetVal = QWindowsUiaMainProvider::providerForAccessible(table); // Detach } return S_OK; } diff --git a/src/plugins/platforms/windows/uiautomation/qwindowsuiagridprovider.cpp b/src/plugins/platforms/windows/uiautomation/qwindowsuiagridprovider.cpp index 9f62ef5fbe6..755b672143b 100644 --- a/src/plugins/platforms/windows/uiautomation/qwindowsuiagridprovider.cpp +++ b/src/plugins/platforms/windows/uiautomation/qwindowsuiagridprovider.cpp @@ -46,7 +46,7 @@ HRESULT STDMETHODCALLTYPE QWindowsUiaGridProvider::GetItem(int row, int column, if ((row >= 0) && (row < tableInterface->rowCount()) && (column >= 0) && (column < tableInterface->columnCount())) { if (QAccessibleInterface *cell = tableInterface->cellAt(row, column)) { - *pRetVal = QWindowsUiaMainProvider::providerForAccessible(cell); + *pRetVal = QWindowsUiaMainProvider::providerForAccessible(cell); // Detach } } return S_OK; diff --git a/src/plugins/platforms/windows/uiautomation/qwindowsuiamainprovider.cpp b/src/plugins/platforms/windows/uiautomation/qwindowsuiamainprovider.cpp index bf44795fe75..b4ac139b0f0 100644 --- a/src/plugins/platforms/windows/uiautomation/qwindowsuiamainprovider.cpp +++ b/src/plugins/platforms/windows/uiautomation/qwindowsuiamainprovider.cpp @@ -79,8 +79,10 @@ void QWindowsUiaMainProvider::notifyFocusChange(QAccessibleEvent *event) if (QAccessibleInterface *child = accessible->focusChild()) accessible = child; } - if (QWindowsUiaMainProvider *provider = providerForAccessible(accessible)) + if (QWindowsUiaMainProvider *provider = providerForAccessible(accessible)) { UiaRaiseAutomationEvent(provider, UIA_AutomationFocusChangedEventId); + provider->Release(); + } } } @@ -98,6 +100,7 @@ void QWindowsUiaMainProvider::notifyStateChange(QAccessibleStateChangeEvent *eve toggleState = accessible->state().checkStateMixed ? ToggleState_Indeterminate : ToggleState_On; setVariantI4(toggleState, &newVal); UiaRaiseAutomationPropertyChangedEvent(provider, UIA_ToggleToggleStatePropertyId, oldVal, newVal); + provider->Release(); } } } @@ -108,12 +111,15 @@ void QWindowsUiaMainProvider::notifyStateChange(QAccessibleStateChangeEvent *eve if (accessible->state().active) { UiaRaiseAutomationEvent(provider, UIA_Window_WindowOpenedEventId); if (QAccessibleInterface *focused = accessible->focusChild()) { - if (QWindowsUiaMainProvider *focusedProvider = providerForAccessible(focused)) + if (QWindowsUiaMainProvider *focusedProvider = providerForAccessible(focused)) { UiaRaiseAutomationEvent(focusedProvider, UIA_AutomationFocusChangedEventId); + focusedProvider->Release(); + } } } else { UiaRaiseAutomationEvent(provider, UIA_Window_WindowClosedEventId); } + provider->Release(); } } } @@ -146,6 +152,8 @@ void QWindowsUiaMainProvider::notifyValueChange(QAccessibleValueChangeEvent *eve clearVariant(&oldVal); setVariantString(event->value().toString(), &newVal); UiaRaiseAutomationPropertyChangedEvent(provider, UIA_ValueValuePropertyId, oldVal, newVal); + provider->Release(); + HRESULT hr = VariantClear(&newVal); // Free string allocated by setVariantString Q_ASSERT(hr == S_OK); Q_UNUSED(hr) @@ -157,6 +165,7 @@ void QWindowsUiaMainProvider::notifyValueChange(QAccessibleValueChangeEvent *eve clearVariant(&oldVal); setVariantDouble(valueInterface->currentValue().toDouble(), &newVal); UiaRaiseAutomationPropertyChangedEvent(provider, UIA_RangeValueValuePropertyId, oldVal, newVal); + provider->Release(); } } } @@ -174,6 +183,7 @@ void QWindowsUiaMainProvider::notifyNameChange(QAccessibleEvent *event) setVariantString(accessible->text(QAccessible::Name), &newVal); UiaRaiseAutomationPropertyChangedEvent(provider, UIA_NamePropertyId, oldVal, newVal); ::SysFreeString(newVal.bstrVal); + provider->Release(); } } } @@ -184,6 +194,7 @@ void QWindowsUiaMainProvider::notifySelectionChange(QAccessibleEvent *event) if (QAccessibleInterface *accessible = event->accessibleInterface()) { if (QWindowsUiaMainProvider *provider = providerForAccessible(accessible)) { UiaRaiseAutomationEvent(provider, UIA_SelectionItem_ElementSelectedEventId); + provider->Release(); } } } @@ -203,6 +214,7 @@ void QWindowsUiaMainProvider::notifyTextChange(QAccessibleEvent *event) } else { UiaRaiseAutomationEvent(provider, UIA_Text_TextChangedEventId); } + provider->Release(); } } } @@ -222,6 +234,7 @@ void QWindowsUiaMainProvider::raiseNotification(QAccessibleAnnouncementEvent *ev ::SysFreeString(message); ::SysFreeString(activityId); + provider->Release(); } } } @@ -697,7 +710,7 @@ HRESULT QWindowsUiaMainProvider::Navigate(NavigateDirection direction, IRawEleme } if (targetacc) - *pRetVal = providerForAccessible(targetacc); + *pRetVal = providerForAccessible(targetacc); // Detach return S_OK; } @@ -786,7 +799,7 @@ HRESULT QWindowsUiaMainProvider::get_FragmentRoot(IRawElementProviderFragmentRoo if (QAccessibleInterface *accessible = accessibleInterface()) { if (QWindow *window = windowForAccessible(accessible)) { if (QAccessibleInterface *rootacc = window->accessibleRoot()) { - *pRetVal = providerForAccessible(rootacc); + *pRetVal = providerForAccessible(rootacc); // Detach } } } @@ -827,7 +840,7 @@ HRESULT QWindowsUiaMainProvider::ElementProviderFromPoint(double x, double y, IR if (targetacc->textInterface()) break; acc = acc->childAt(point.x(), point.y()); } - *pRetVal = providerForAccessible(targetacc); + *pRetVal = providerForAccessible(targetacc); // Detach } return S_OK; } @@ -843,7 +856,7 @@ HRESULT QWindowsUiaMainProvider::GetFocus(IRawElementProviderFragment **pRetVal) if (QAccessibleInterface *accessible = accessibleInterface()) { if (QAccessibleInterface *focusacc = accessible->focusChild()) { - *pRetVal = providerForAccessible(focusacc); + *pRetVal = providerForAccessible(focusacc); // Detach } } return S_OK; diff --git a/src/plugins/platforms/windows/uiautomation/qwindowsuiaselectionitemprovider.cpp b/src/plugins/platforms/windows/uiautomation/qwindowsuiaselectionitemprovider.cpp index 95bc2f7570b..a7fc55d5a76 100644 --- a/src/plugins/platforms/windows/uiautomation/qwindowsuiaselectionitemprovider.cpp +++ b/src/plugins/platforms/windows/uiautomation/qwindowsuiaselectionitemprovider.cpp @@ -178,7 +178,7 @@ HRESULT STDMETHODCALLTYPE QWindowsUiaSelectionItemProvider::get_SelectionContain QAccessibleInterface *parent = accessible->parent(); if (parent && parent->selectionInterface()) { - *pRetVal = QWindowsUiaMainProvider::providerForAccessible(parent); + *pRetVal = QWindowsUiaMainProvider::providerForAccessible(parent); // Detach return S_OK; } @@ -190,7 +190,7 @@ HRESULT STDMETHODCALLTYPE QWindowsUiaSelectionItemProvider::get_SelectionContain if (parent) { if ((accessible->role() == QAccessible::ListItem && parent->role() == QAccessible::List) || (accessible->role() == QAccessible::PageTab && parent->role() == QAccessible::PageTabList)) { - *pRetVal = QWindowsUiaMainProvider::providerForAccessible(parent); + *pRetVal = QWindowsUiaMainProvider::providerForAccessible(parent); // Detach } } return S_OK; diff --git a/src/plugins/platforms/windows/uiautomation/qwindowsuiaselectionprovider.cpp b/src/plugins/platforms/windows/uiautomation/qwindowsuiaselectionprovider.cpp index 37148c655a8..7e9d0151e25 100644 --- a/src/plugins/platforms/windows/uiautomation/qwindowsuiaselectionprovider.cpp +++ b/src/plugins/platforms/windows/uiautomation/qwindowsuiaselectionprovider.cpp @@ -163,7 +163,7 @@ HRESULT STDMETHODCALLTYPE QWindowsUiaSelectionProvider::get_FirstSelectedItem(__ if (QWindowsUiaMainProvider *childProvider = QWindowsUiaMainProvider::providerForAccessible(firstSelectedChild)) { - *pRetVal = static_cast(childProvider); + *pRetVal = static_cast(childProvider); // Detach return S_OK; } @@ -208,7 +208,7 @@ HRESULT STDMETHODCALLTYPE QWindowsUiaSelectionProvider::get_LastSelectedItem(__R if (QWindowsUiaMainProvider *childProvider = QWindowsUiaMainProvider::providerForAccessible(lastSelectedChild)) { - *pRetVal = static_cast(childProvider); + *pRetVal = static_cast(childProvider); // Detach return S_OK; } diff --git a/src/plugins/platforms/windows/uiautomation/qwindowsuiatextrangeprovider.cpp b/src/plugins/platforms/windows/uiautomation/qwindowsuiatextrangeprovider.cpp index a62a33cfe2b..716c2e8e15a 100644 --- a/src/plugins/platforms/windows/uiautomation/qwindowsuiatextrangeprovider.cpp +++ b/src/plugins/platforms/windows/uiautomation/qwindowsuiatextrangeprovider.cpp @@ -265,7 +265,7 @@ HRESULT QWindowsUiaTextRangeProvider::GetEnclosingElement(IRawElementProviderSim if (!accessible) return UIA_E_ELEMENTNOTAVAILABLE; - *pRetVal = QWindowsUiaMainProvider::providerForAccessible(accessible); + *pRetVal = QWindowsUiaMainProvider::providerForAccessible(accessible); // Detach return S_OK; }