mir/libreoffice
2
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

valueset a11y: Dispose ValueItemAcc objects

Browse Source 
So far, the ValueItemAcc objects created by
ValueSet were never disposed.

While this is a preexisting issue, this started triggering
crashes with the qt6 VCL plugin after

    Change-Id: If448008b3a6dc7b22a06b6ed551b08a40b2d5de3
    Author: Michael Weghorn <m.weghorn@posteo.de>
    Date:   Tue Feb 25 12:14:24 2025 +0100

        valueset a11y: Use OAccessibleComponentHelper for ValueItemAcc

as described in its commit message.

Fix this by disposing the objects in ValueSet::ImplDeleteItems,
and not just sending an AccessibleEventId::CHILD event.

Adjust the logic to be independent of whether the item is visible,
but instead do this for all items that have an associated ValueItemAcc.
Add a new `bCreate` param to ValueSetItem::GetAccessible and pass
false here to avoid creating new ones.

In the ValueSet dtor, call `ImplDeleteItems` before invalidating
the accessible object, as it may still be needed in `ImplDeleteItems`
to send the events.

This fixes the crash on exit for the scenario described in the
above-mentioned commit.

Backtrace of such a crash/assert:

    soffice.bin: /home/michi/development/git/libreoffice/comphelper/source/misc/accessibleeventnotifier.cxx:142: bool (anonymous namespace)::implLookupClient(const AccessibleEventNotifier::TClientId, ClientMap::iterator &): Assertion `rClients.end() != rPos && "AccessibleEventNotifier::implLookupClient: invalid client id " "(did you register your client?)!"' failed.
    [New Thread 9546.9547]
    [New Thread 9546.9548]
    [New Thread 9546.9549]
    [New Thread 9546.9557]

    Thread 1 received signal SIGABRT, Aborted.
    __pthread_kill_implementation (threadid=<optimized out>, signo=signo@entry=6, no_tid=no_tid@entry=0) at ./nptl/pthread_kill.c:44
    warning: 44     ./nptl/pthread_kill.c: No such file or directory
    (rr) bt
    #0  __pthread_kill_implementation (threadid=<optimized out>, signo=signo@entry=6, no_tid=no_tid@entry=0) at ./nptl/pthread_kill.c:44
    #1  0x00007ff8c289de2f in __pthread_kill_internal (threadid=<optimized out>, signo=6) at ./nptl/pthread_kill.c:78
    #2  0x00007ff8c2849d02 in __GI_raise (sig=sig@entry=6) at ../sysdeps/posix/raise.c:26
    #3  0x00007ff8c28324f0 in __GI_abort () at ./stdlib/abort.c:79
    #4  0x00007ff8c2832418 in __assert_fail_base
        (fmt=0x7ff8c29b6ca0 "%s%s%s:%u: %s%sAssertion `%s' failed.\n%n", assertion=assertion@entry=0x7ff8c0f27cc6 "rClients.end() != rPos && \"AccessibleEventNotifier::implLookupClient: invalid client id \" \"(did you register your client?)!\"", file=file@entry=0x7ff8c0f360b9 "/home/michi/development/git/libreoffice/comphelper/source/misc/accessibleeventnotifier.cxx", line=line@entry=142, function=function@entry=0x7ff8c0efd262 "bool (anonymous namespace)::implLookupClient(const AccessibleEventNotifier::TClientId, ClientMap::iterator &)") at ./assert/assert.c:96
    #5  0x00007ff8c2842612 in __assert_fail
        (assertion=0x7ff8c0f27cc6 "rClients.end() != rPos && \"AccessibleEventNotifier::implLookupClient: invalid client id \" \"(did you register your client?)!\"", file=0x7ff8c0f360b9 "/home/michi/development/git/libreoffice/comphelper/source/misc/accessibleeventnotifier.cxx", line=142, function=0x7ff8c0efd262 "bool (anonymous namespace)::implLookupClient(const AccessibleEventNotifier::TClientId, ClientMap::iterator &)") at ./assert/assert.c:105
    #6  0x00007ff8c10eb4c8 in (anonymous namespace)::implLookupClient (nClient=24, rPos=...) at /home/michi/development/git/libreoffice/comphelper/source/misc/accessibleeventnotifier.cxx:140
    #7  0x00007ff8c10eb938 in comphelper::AccessibleEventNotifier::revokeClientNotifyDisposing (_nClient=24, _rxEventSource=uno::Reference to (ValueItemAcc *) 0x558da6f7a220)
        at /home/michi/development/git/libreoffice/comphelper/source/misc/accessibleeventnotifier.cxx:185
    #8  0x00007ff8c10e751d in comphelper::OCommonAccessibleComponent::disposing (this=0x558da6f7a220) at /home/michi/development/git/libreoffice/comphelper/source/misc/accessiblecomponenthelper.cxx:61
    #9  0x00007ff8c0b3cee0 in cppu::WeakComponentImplHelperBase::dispose (this=0x558da6f7a220) at /home/michi/development/git/libreoffice/cppuhelper/source/implbase.cxx:104
    #10 0x00007ff8bb84a0c5 in cppu::PartialWeakComponentImplHelper<com::sun:⭐:accessibility::XAccessibleContext2, com::sun:⭐:accessibility::XAccessibleEventBroadcaster>::dispose (this=0x558da6f7a220)
        at include/cppuhelper/compbase.hxx:90
    #11 0x00007ff8c0b3cc07 in cppu::WeakComponentImplHelperBase::release (this=0x558da6f7a220) at /home/michi/development/git/libreoffice/cppuhelper/source/implbase.cxx:79
    #12 0x00007ff8bb84d6c5 in cppu::PartialWeakComponentImplHelper<com::sun:⭐:accessibility::XAccessibleContext2, com::sun:⭐:accessibility::XAccessibleEventBroadcaster>::release (this=0x558da6f7a220)
        at include/cppuhelper/compbase.hxx:86
    #13 0x00007ff8bb8e0b95 in cppu::ImplInheritanceHelper<comphelper::OCommonAccessibleComponent, com::sun:⭐:accessibility::XAccessibleComponent>::release (this=0x558da6f7a220) at include/cppuhelper/implbase.hxx:171
    #14 0x00007ff8bb97d595 in cppu::ImplInheritanceHelper<comphelper::OAccessibleComponentHelper, com::sun:⭐:accessibility::XAccessible>::release (this=0x558da6f7a220) at include/cppuhelper/implbase.hxx:171
    #15 0x00007ff8af309baa in com::sun:⭐:uno::Reference<com::sun:⭐:accessibility::XAccessible>::~Reference (this=0x558da6f7a428) at include/com/sun/star/uno/Reference.hxx:114
    #16 0x00007ff8af321bad in QtAccessibleWidget::~QtAccessibleWidget (this=0x558da6f7a3e0) at vcl/inc/qt6/../qt5/QtAccessibleWidget.hxx:39
    #17 0x00007ff8af321c49 in QtAccessibleWidget::~QtAccessibleWidget (this=0x558da6f7a3e0) at vcl/inc/qt6/../qt5/QtAccessibleWidget.hxx:39
    #18 0x00007ff8ad8faf48 in QAccessibleCache::deleteInterface (this=0x558d9fbd0ac0, id=2147483692, obj=0x558da6eecdb0) at /home/michi/development/git/qt5/qtbase/src/gui/accessible/qaccessiblecache.cpp:173
    #19 0x00007ff8ad8fad78 in QAccessibleCache::~QAccessibleCache (this=0x558d9fbd0ac0) at /home/michi/development/git/qt5/qtbase/src/gui/accessible/qaccessiblecache.cpp:31
    #20 0x00007ff8ad8faf8d in QAccessibleCache::~QAccessibleCache (this=0x558d9fbd0ac0) at /home/michi/development/git/qt5/qtbase/src/gui/accessible/qaccessiblecache.cpp:29
    #21 0x00007ff8ad8fb027 in cleanupAccessibleCache () at /home/michi/development/git/qt5/qtbase/src/gui/accessible/qaccessiblecache.cpp:24
    #22 0x00007ff8ae44a792 in qt_call_post_routines () at /home/michi/development/git/qt5/qtbase/src/corelib/kernel/qcoreapplication.cpp:343
    #23 0x00007ff8ac3ddee4 in QApplication::~QApplication (this=0x558d9e8c76c0) at /home/michi/development/git/qt5/qtbase/src/widgets/kernel/qapplication.cpp:667
    #24 0x00007ff8ac3de29d in QApplication::~QApplication (this=0x558d9e8c76c0) at /home/michi/development/git/qt5/qtbase/src/widgets/kernel/qapplication.cpp:663
    #25 0x00007ff8af3d63b8 in std::default_delete<QApplication>::operator() (this=0x558d9e955870, __ptr=0x558d9e8c76c0) at /usr/lib/gcc/x86_64-linux-gnu/14/../../../../include/c++/14/bits/unique_ptr.h:93
    #26 0x00007ff8af3d7da8 in std::__uniq_ptr_impl<QApplication, std::default_delete<QApplication> >::reset (this=0x558d9e955870, __p=0x0) at /usr/lib/gcc/x86_64-linux-gnu/14/../../../../include/c++/14/bits/unique_ptr.h:205
    #27 0x00007ff8af3cfcbd in std::unique_ptr<QApplication, std::default_delete<QApplication> >::reset (this=0x558d9e955870, __p=0x0) at /usr/lib/gcc/x86_64-linux-gnu/14/../../../../include/c++/14/bits/unique_ptr.h:504
    #28 0x00007ff8af3c7b34 in QtInstance::~QtInstance (this=0x558d9e9556e0) at vcl/qt6/../qt5/QtInstance.cxx:323
    #29 0x00007ff8af3c7c29 in QtInstance::~QtInstance (this=0x558d9e9556e0) at vcl/qt6/../qt5/QtInstance.cxx:320
    #30 0x00007ff8b9bb1c54 in DestroySalInstance (pInst=0x558d9e9556f0) at /home/michi/development/git/libreoffice/vcl/source/app/salplug.cxx:361
    #31 0x00007ff8b9ca1509 in DeInitVCL () at /home/michi/development/git/libreoffice/vcl/source/app/svmain.cxx:629
    #32 0x00007ff8b9c9fa4f in ImplSVMain () at /home/michi/development/git/libreoffice/vcl/source/app/svmain.cxx:241
    #33 0x00007ff8b9ca15e9 in SVMain () at /home/michi/development/git/libreoffice/vcl/source/app/svmain.cxx:248
    #34 0x00007ff8c2b9f4ba in soffice_main () at /home/michi/development/git/libreoffice/desktop/source/app/sofficemain.cxx:122
    #35 0x0000558d9330ba6d in sal_main () at /home/michi/development/git/libreoffice/desktop/source/app/main.c:51
    #36 0x0000558d9330ba47 in main (argc=2, argv=0x7ffd2669fb48) at /home/michi/development/git/libreoffice/desktop/source/app/main.c:49

Change-Id: Ifa7e18393edcc1889bcb390fa453c611d9345bdc
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/182174
Tested-by: Jenkins
Reviewed-by: Michael Weghorn <m.weghorn@posteo.de>
This commit is contained in:
Michael Weghorn
2025-02-25 12:33:21 +01:00
parent e7dc7ebc9e
commit b3c460980c
3 changed files with 15 additions and 11 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
svtools/source/control/valueacc.cxx
View File

@@ -54,9 +54,9 @@ ValueSetItem::~ValueSetItem()
}
}
const rtl::Reference< ValueItemAcc > & ValueSetItem::GetAccessible()
const rtl::Reference<ValueItemAcc>& ValueSetItem::GetAccessible(bool bCreate)
{
if( !mxAcc.is() )
if (!mxAcc.is() && bCreate)
mxAcc = new ValueItemAcc(this);
return mxAcc;

2
svtools/source/control/valueimp.hxx
View File

@@ -59,7 +59,7 @@ struct ValueSetItem
explicit ValueSetItem( ValueSet& rParent );
~ValueSetItem();
const rtl::Reference< ValueItemAcc > & GetAccessible();
const rtl::Reference<ValueItemAcc>& GetAccessible(bool bCreate = true);
};
class ValueSetAcc final

20
svtools/source/control/valueset.cxx
View File

@@ -125,10 +125,10 @@ Reference<XAccessible> ValueSet::CreateAccessible()
ValueSet::~ValueSet()
{
ImplDeleteItems();
if (mxAccessible)
mxAccessible->Invalidate();
ImplDeleteItems();
}
void ValueSet::ImplDeleteItems()
@@ -137,14 +137,18 @@ void ValueSet::ImplDeleteItems()
for ( size_t i = 0; i < n; ++i )
{
ValueSetItem* pItem = mItemList[i].get();
if ( pItem->mbVisible && ImplHasAccessibleListeners() )
if (ValueSetItem* pItem = mItemList[i].get())
{
Any aOldAny;
Any aNewAny;
rtl::Reference<ValueItemAcc> xItemAcc = pItem->GetAccessible(false);
if (xItemAcc.is())
{
Any aOldAny;
Any aNewAny;
aOldAny <<= Reference<XAccessible>(xItemAcc);
ImplFireAccessibleEvent(AccessibleEventId::CHILD, aOldAny, aNewAny);
aOldAny <<= Reference< XAccessible >(pItem->GetAccessible());
ImplFireAccessibleEvent(AccessibleEventId::CHILD, aOldAny, aNewAny);
xItemAcc->dispose();
}
}
mItemList[i].reset();