Bug 135330 - With Screen Reader NVDA LibreOffice Crashes if a Word Count is Performed
Summary: With Screen Reader NVDA LibreOffice Crashes if a Word Count is Performed
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Writer (show other bugs)
Version:
(earliest affected)
6.2.0.3 release
Hardware: All Windows (All)
: medium normal
Assignee: Not Assigned
URL:
Whiteboard: target:7.1.0
Keywords: accessibility, bibisected, bisected, regression
Depends on:
Blocks: a11y, Accessibility Word-Count
  Show dependency treegraph
 
Reported: 2020-07-30 19:55 UTC by Harald Koester
Modified: 2023-03-29 07:32 UTC (History)
7 users (show)

See Also:
Crash report or crash signature: crashreport.libreoffice.org/stats/crash_details/db44f0f2-71fe-4c3b-956a-a59c1b85b07d,crashreport.libreoffice.org/stats/crash_details/4aaa134c-5019-4d76-b130-c64dcbe139b5


Attachments
crash with NVDA screen reader installed and sounding LO 7.0.0rc2 on use of Word Count tool (32.37 KB, text/plain)
2020-07-30 21:02 UTC, V Stuart Foote
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Harald Koester 2020-07-30 19:55:54 UTC
In order to reproduce the bug:
[1] In your browser go to http://meinnvda.de/?lang=en
[2] Click “Start NVDA now”. Save mynvda.exe on your system and start it. The screen reader is now activated.
[3] Start LibreOffice and create a new text document.
[4] Insert a few words.
[5] Perform Word Count from the menu: Tools > Word Count … LibreOffice crashes. The recovery dialogue is displayed.

The bug occured first in version 6.2.0 release. It still exists in version 6.4.5.

Crash Report:
crashreport.libreoffice.org/stats/crash_details/db44f0f2-71fe-4c3b-956a-a59c1b85b07d
Comment 1 Karl-Heinz Arkenau 2020-07-30 20:43:54 UTC
Happens also on Windows 10 with LibreOffice 6.4.5.2 with Screenreader NVDA 2020.2.
Comment 2 V Stuart Foote 2020-07-30 21:02:56 UTC
Created attachment 163793 [details]
crash with NVDA screen reader installed and sounding LO 7.0.0rc2 on use of Word Count tool

Version: 7.0.0.2 (x64)
Build ID: c01aa64b6c3d89ebe5fe69c28c7adb24eb85249c
CPU threads: 8; OS: Windows 10.0 Build 18363; UI render: Skia/Vulkan; VCL: win
Locale: en-US (en_US); UI: en-US
Calc: CL
Comment 3 V Stuart Foote 2020-07-30 21:09:08 UTC
Confirmed on Windows 10 Ent 64-bit en-US (1909) with NVDA 2019.3.1 full installed

Stack trace of crash attached. 

Uploaded to Crashreport server as:
https://crashreport.libreoffice.org/stats/crash_details/4aaa134c-5019-4d76-b130-c64dcbe139b5
Comment 4 Julien Nabet 2020-07-31 08:07:31 UTC
Mike: taking a look at the crashreport, I wonder if your fix for tdf#135244 put in see also could help here.
Comment 5 Buovjaga 2020-08-01 17:14:35 UTC
Bibisected with win32-6.2 to https://git.libreoffice.org/core/commit/26c375671aa362b2f59d84645784938677ae1719
weld SwWordCountFloatDlg

Adding Cc: to Caolán McNamara
Comment 6 Mike Kaganski 2020-08-01 17:55:24 UTC
Mutex is 0xdddddddddddddddd:

sal3.dll!osl_acquireMutex(_oslMutexImpl * Mutex) Line 66
	at C:\lo\src\core\sal\osl\w32\mutex.cxx(66)
utllo.dll!osl::Mutex::acquire() Line 57
	at C:\lo\src\core\include\osl\mutex.hxx(57)
utllo.dll!osl::Guard<osl::Mutex>::Guard<osl::Mutex>(osl::Mutex & t) Line 136
	at C:\lo\src\core\include\osl\mutex.hxx(136)
utllo.dll!utl::ReadWriteGuard::ReadWriteGuard(utl::ReadWriteMutex & rMutexP, ReadWriteGuardMode nRequestMode) Line 31
	at C:\lo\src\core\unotools\source\i18n\readwritemutexguard.cxx(31)
utllo.dll!LocaleDataWrapper::getNum(__int64 nNumber, unsigned short nDecimals, bool bUseThousandSep, bool bTrailingZeros) Line 1443
	at C:\lo\src\core\unotools\source\i18n\localedatawrapper.cxx(1443)
swuilo.dll!`anonymous namespace'::setValue(weld::Label & rWidget, unsigned __int64 nValue, const LocaleDataWrapper & rLocaleData) Line 47
	at C:\lo\src\core\sw\source\ui\dialog\wordcountdialog.cxx(47)
swuilo.dll!SwWordCountFloatDlg::SetValues(const SwDocStat & rCurrent, const SwDocStat & rDoc) Line 62
	at C:\lo\src\core\sw\source\ui\dialog\wordcountdialog.cxx(62)
swuilo.dll!SwWordCountFloatDlg::UpdateCounts() Line 146
	at C:\lo\src\core\sw\source\ui\dialog\wordcountdialog.cxx(146)
swuilo.dll!AbstractSwWordCountFloatDlg_Impl::UpdateCounts() Line 748
	at C:\lo\src\core\sw\source\ui\dialog\swdlgfact.cxx(748)
swlo.dll!SwWordCountWrapper::UpdateCounts() Line 41
	at C:\lo\src\core\sw\source\uibase\dialog\wordcountwrapper.cxx(41)
swlo.dll!SwView::UpdateWordCount(SfxShell * pShell, unsigned short nSlot) Line 215
	at C:\lo\src\core\sw\source\uibase\uiview\view1.cxx(215)
swlo.dll!SwTextShell::Execute(SfxRequest & rReq) Line 1392
	at C:\lo\src\core\sw\source\uibase\shells\textsh1.cxx(1392)
...

which means that the mutex in rLocaleData, which was already successfully used in SwWordCountFloatDlg::SetValues for m_xCurrentWordFT, got deleted (hence 0xdddddddddddddddd in debug build) and crashes when processing m_xCurrentCharacterFT ...
Comment 7 Mike Kaganski 2020-08-01 20:30:40 UTC
... and LocaleDataWrapper is deleted in

utllo.dll!LocaleDataWrapper::~LocaleDataWrapper() Line 111
	at C:\lo\src\core\unotools\source\i18n\localedatawrapper.cxx(111)
vcllo.dll!LocaleDataWrapper::`scalar deleting destructor'(unsigned int)
vcllo.dll!std::default_delete<LocaleDataWrapper>::operator()(LocaleDataWrapper * _Ptr) Line 1762
	at C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\MSVC\14.26.28801\include\memory(1762)
vcllo.dll!std::unique_ptr<LocaleDataWrapper,std::default_delete<LocaleDataWrapper>>::reset(LocaleDataWrapper * _Ptr) Line 1914
	at C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\MSVC\14.26.28801\include\memory(1914)
vcllo.dll!ImplAllSettingsData::~ImplAllSettingsData() Line 2759
	at C:\lo\src\core\vcl\source\app\settings.cxx(2759)
vcllo.dll!ImplAllSettingsData::`scalar deleting destructor'(unsigned int)
vcllo.dll!std::_Destroy_in_place<ImplAllSettingsData>(ImplAllSettingsData & _Obj) Line 243
	at C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\MSVC\14.26.28801\include\xmemory(243)
vcllo.dll!std::_Ref_count_obj2<ImplAllSettingsData>::_Destroy() Line 1504
	at C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\MSVC\14.26.28801\include\memory(1504)
vcllo.dll!std::_Ref_count_base::_Decref() Line 651
	at C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\MSVC\14.26.28801\include\memory(651)
vcllo.dll!std::_Ptr_base<ImplAllSettingsData>::_Decref() Line 883
	at C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\MSVC\14.26.28801\include\memory(883)
vcllo.dll!std::shared_ptr<ImplAllSettingsData>::~shared_ptr<ImplAllSettingsData>() Line 1132
	at C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\MSVC\14.26.28801\include\memory(1132)
vcllo.dll!AllSettings::~AllSettings()
vcllo.dll!Application::SetSettings(const AllSettings & rSettings) Line 712
	at C:\lo\src\core\vcl\source\app\svapp.cxx(712)
vclplug_winlo.dll!ImplHandleGetObject(HWND__ * hWnd, __int64 lParam, unsigned __int64 wParam, __int64 & nRet) Line 5301
	at C:\lo\src\core\vcl\win\window\salframe.cxx(5301)
vclplug_winlo.dll!SalFrameWndProc(HWND__ * hWnd, unsigned int nMsg, unsigned __int64 wParam, __int64 lParam, bool & rDef) Line 5846
	at C:\lo\src\core\vcl\win\window\salframe.cxx(5846)
vclplug_winlo.dll!SalFrameWndProcW(HWND__ * hWnd, unsigned int nMsg, unsigned __int64 wParam, __int64 lParam) Line 5893
	at C:\lo\src\core\vcl\win\window\salframe.cxx(5893)
user32.dll!UserCallWinProcCheckWow()
user32.dll!DispatchClientMessage()
user32.dll!__fnDWORD()
ntdll.dll!KiUserCallbackDispatcherContinue()
win32u.dll!NtUserMessageCall()
user32.dll!SendMessageTimeoutW()
oleacc.dll!NativeIAccessibleFromWindow()
oleacc.dll!ORIGINAL_AccessibleObjectFromWindow()
oleacc.dll!AccessibleObjectFromWindow()
oleacc.dll!AccessibleObjectFromEvent()
nvdaHelperRemote.dll!00007ffde1fad2b2()
nvdaHelperRemote.dll!00007ffde1f938ad()
user32.dll!__ClientCallWinEventProc()
ntdll.dll!KiUserCallbackDispatcherContinue()
win32u.dll!NtUserNotifyWinEvent()
user32.dll!NotifyWinEvent()
winaccessibility.dll!AccObjectWinManager::NotifyAccEvent(com::sun::star::accessibility::XAccessible * pXAcc, short state) Line 237
	at C:\lo\src\core\winaccessibility\source\service\AccObjectWinManager.cxx(237)
winaccessibility.dll!AccObjectManagerAgent::NotifyAccEvent(short pEvent, com::sun::star::accessibility::XAccessible * pXAcc) Line 252
	at C:\lo\src\core\winaccessibility\source\service\AccObjectManagerAgent.cxx(252)
winaccessibility.dll!AccEventListener::HandleNameChangedEvent(com::sun::star::uno::Any name) Line 96
	at C:\lo\src\core\winaccessibility\source\service\AccEventListener.cxx(96)
winaccessibility.dll!AccEventListener::notifyEvent(const com::sun::star::accessibility::AccessibleEventObject & aEvent) Line 67
	at C:\lo\src\core\winaccessibility\source\service\AccEventListener.cxx(67)
winaccessibility.dll!AccComponentEventListener::notifyEvent(const com::sun::star::accessibility::AccessibleEventObject & aEvent) Line 82
	at C:\lo\src\core\winaccessibility\source\service\AccComponentEventListener.cxx(82)
comphelper.dll!comphelper::AccessibleEventNotifier::addEvent(const unsigned long _nClient, const com::sun::star::accessibility::AccessibleEventObject & _rEvent) Line 268
	at C:\lo\src\core\comphelper\source\misc\accessibleeventnotifier.cxx(268)
comphelper.dll!comphelper::OAccessibleContextHelper::NotifyAccessibleEvent(const short _nEventId, const com::sun::star::uno::Any & _rOldValue, const com::sun::star::uno::Any & _rNewValue) Line 159
	at C:\lo\src\core\comphelper\source\misc\accessiblecontexthelper.cxx(159)
tklo.dll!VCLXAccessibleComponent::ProcessWindowEvent(const VclWindowEvent & rVclWindowEvent) Line 264
	at C:\lo\src\core\toolkit\source\awt\vclxaccessiblecomponent.cxx(264)
acclo.dll!VCLXAccessibleTextComponent::ProcessWindowEvent(const VclWindowEvent & rVclWindowEvent) Line 72
	at C:\lo\src\core\accessibility\source\standard\vclxaccessibletextcomponent.cxx(72)
tklo.dll!VCLXAccessibleComponent::WindowEventListener(VclWindowEvent & rEvent) Line 115
	at C:\lo\src\core\toolkit\source\awt\vclxaccessiblecomponent.cxx(115)
tklo.dll!VCLXAccessibleComponent::LinkStubWindowEventListener(void * instance, VclWindowEvent & data) Line 101
	at C:\lo\src\core\toolkit\source\awt\vclxaccessiblecomponent.cxx(101)
vcllo.dll!Link<VclWindowEvent &,void>::Call(VclWindowEvent & data) Line 111
	at C:\lo\src\core\include\tools\link.hxx(111)
vcllo.dll!vcl::Window::CallEventListeners(VclEventId nEvent, void * pData) Line 257
	at C:\lo\src\core\vcl\source\window\event.cxx(257)
vcllo.dll!vcl::Window::SetText(const rtl::OUString & rStr) Line 3075
	at C:\lo\src\core\vcl\source\window\window.cxx(3075)
vcllo.dll!Control::SetText(const rtl::OUString & rStr) Line 99
	at C:\lo\src\core\vcl\source\control\ctrl.cxx(99)
vcllo.dll!SalInstanceLabel::set_label(const rtl::OUString & rText) Line 5464
	at C:\lo\src\core\vcl\source\app\salvtables.cxx(5464)
swuilo.dll!`anonymous namespace'::setValue(weld::Label & rWidget, unsigned __int64 nValue, const LocaleDataWrapper & rLocaleData) Line 47
	at C:\lo\src\core\sw\source\ui\dialog\wordcountdialog.cxx(47)
swuilo.dll!SwWordCountFloatDlg::SetValues(const SwDocStat & rCurrent, const SwDocStat & rDoc) Line 61
	at C:\lo\src\core\sw\source\ui\dialog\wordcountdialog.cxx(61)
swuilo.dll!SwWordCountFloatDlg::UpdateCounts() Line 146
	at C:\lo\src\core\sw\source\ui\dialog\wordcountdialog.cxx(146)
swuilo.dll!AbstractSwWordCountFloatDlg_Impl::UpdateCounts() Line 748
	at C:\lo\src\core\sw\source\ui\dialog\swdlgfact.cxx(748)
swlo.dll!SwWordCountWrapper::UpdateCounts() Line 41
	at C:\lo\src\core\sw\source\uibase\dialog\wordcountwrapper.cxx(41)
swlo.dll!SwView::UpdateWordCount(SfxShell * pShell, unsigned short nSlot) Line 215
	at C:\lo\src\core\sw\source\uibase\uiview\view1.cxx(215)
swlo.dll!SwTextShell::Execute(SfxRequest & rReq) Line 1392
	at C:\lo\src\core\sw\source\uibase\shells\textsh1.cxx(1392)
...

Why does accessibility cause Application::SetSettings - which looks like deleting current AllSettings object, and LocaleDataWrapper among other things...
Comment 8 Mike Kaganski 2020-08-01 21:06:30 UTC
> static bool
> ImplHandleGetObject(HWND hWnd, LPARAM lParam, WPARAM wParam, LRESULT & nRet)
> {
>     // IA2 should be enabled automatically
>     AllSettings aSettings = Application::GetSettings();
>     MiscSettings aMisc = aSettings.GetMiscSettings();
>     aMisc.SetEnableATToolSupport( true );
>     aSettings.SetMiscSettings( aMisc );
>     Application::SetSettings( aSettings );
>     ...

So I see a bunch of problems here. Some background:

AllSettings has a shared pointer to ImplAllSettingsData.
ImplAllSettingsData includes a MiscSettings, which has a shared pointer to ImplMiscData.

So in the end, aMisc has a shared pointer to the same ImplMiscData as used in Application::GetSettings. So aMisc.SetEnableATToolSupport, although looks like operating on a "copy" of a const reference returned by those const member functions Application::GetSettings and AllSettings::GetMiscSettings, in fact changes the shared state. Is this intentional? Not here, but elsewhere, where a similar thing might happen?

Then AllSettings::SetMiscSettings calls CopyData, which checks use count, which is naturally >1 (we have the local object in addition to the global one, both referencing same data), and duplicates everything. Then in Application::SetSettings, the copy will get , and original will be destroyed ... just because we changed true to true.

Because we do that unconditionally - even if Application::GetSettings().GetMiscSettings().GetEnableATToolSupport() is already true. Why?

So one sure thing is to add a check before this; but how to proceed further? should I keep this expensive replacement of all settings, or should I make use of this implementation detail that aMisc.SetEnableATToolSupport already did all I needed?
Comment 9 Mike Kaganski 2020-08-01 21:16:35 UTC
And anyway, I feel uneasy about the fact that this makes clear: using LocaleDataWrapper (and who knows what else) is not safe, because at any moment a call to Application::SetSettings might make the objects invalid.
Comment 10 Mike Kaganski 2020-08-02 07:53:33 UTC
And maybe notification of accessible event should not be synchronous? so that it fires in an idle event?
Comment 11 Commit Notification 2020-08-05 10:05:43 UTC
Mike Kaganski committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/b094741e1d04f4e377c4a7d5c564e38d2f84c377

tdf#135330: avoid replacing all settings when enabling IA2

It will be available in 7.1.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 12 QA Administrators 2022-08-07 03:31:13 UTC Comment hidden (obsolete)
Comment 13 Michael Weghorn 2023-03-23 11:41:15 UTC
(In reply to Commit Notification from comment #11)
> Mike Kaganski committed a patch related to this issue.
> It has been pushed to "master":
> 
> https://git.libreoffice.org/core/commit/
> b094741e1d04f4e377c4a7d5c564e38d2f84c377
> 
> tdf#135330: avoid replacing all settings when enabling IA2
> 
> [...]

Is this still an issue with the above commit in place or can this ticket be closed as fixed?
In a quick test of mine, I no longer see any crash, but can reproduce if I go back to a version that doesn't have that commit, s.a. my comment in the corresponding NVDA issue:
https://github.com/nvaccess/nvda/issues/11455#issuecomment-1481034206

In that NVDA issue, there's also a comment about a still occuring crash, but it's unclear to me whether that is the same issue:
https://github.com/nvaccess/nvda/issues/11455#issuecomment-1463036097
(s.a. my question there in the hope to get more details)
Comment 14 Michael Weghorn 2023-03-29 07:32:21 UTC
(In reply to Michael Weghorn from comment #13)
> Is this still an issue with the above commit in place or can this ticket be
> closed as fixed?
> In a quick test of mine, I no longer see any crash, but can reproduce if I
> go back to a version that doesn't have that commit, s.a. my comment in the
> corresponding NVDA issue:
> https://github.com/nvaccess/nvda/issues/11455#issuecomment-1481034206
> 
> In that NVDA issue, there's also a comment about a still occuring crash, but
> it's unclear to me whether that is the same issue:
> https://github.com/nvaccess/nvda/issues/11455#issuecomment-1463036097
> (s.a. my question there in the hope to get more details)

The issue mentioned in the NVDA issue turned out to be unrelated and the NVDA issue has been closed accordingly. Closing this LO ticket as fixed as well. Please reopen and mention more details if the original issue is still reproducible for you.