vcl/win/window/salframe.cxx | 52 +++++++++++++++++++++++++++-----------------
1 file changed, 32 insertions(+), 20 deletions(-)
New commits:
commit 889c12e98e04edb4bc25b86bf16b8cf1d9b68420
Author: Michael Stahl <michael.st...@allotropia.de>
AuthorDate: Tue Jun 13 12:30:44 2023 +0200
Commit: Michael Stahl <michael.st...@allotropia.de>
CommitDate: Tue Jun 13 16:07:15 2023 +0200
tdf#155794 vcl: handle WM_GETOBJECT without SolarMutex
SalFrameWndProc() handles WM_GETOBJECT by acquiring SolarMutex and
calling ImplHandleGetObject(), which again acquires the SolarMutex
inside Application::SetSettings().
This was introduced with commit db214684057e3ff2fa32d57c00507309dd6c24d6
due to thread-safety crashes but it turns out that it can be problematic.
When loading a document on a non-main thread, WinSalFrame::SetTitle()
calls SetWindowTextW which is equivalent to SendMessage(WM_SETTEXT),
while holding SolarMutex, and if the main thread doesn't finish
processing it then that's a deadlock.
Typically Desktop::Main() has already created the mxAccessBridge, so
ImplHandleGetObject() most likely doesn't need to do it, so just skip
the Settings code there in case the SolarMutex is locked by another
thread.
In case the SolarMutex is locked by another thread, do an unsafe read of
ImplGetSVData()->mxAccessBridge - this should work until ImplSVData is
deleted, by which time no Windows should exist anymore that could be
receiving messages.
This fixes part of the problem, winaccessibility also needs to stop
using SolarMutex.
Change-Id: I62b027ad06d2c3eb06a5f64b052a4acd0908f79c
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/152958
Tested-by: Jenkins
Reviewed-by: Michael Stahl <michael.st...@allotropia.de>
diff --git a/vcl/win/window/salframe.cxx b/vcl/win/window/salframe.cxx
index e0a99d8e8779..2e60f38c1f11 100644
--- a/vcl/win/window/salframe.cxx
+++ b/vcl/win/window/salframe.cxx
@@ -5395,30 +5395,43 @@ static void ImplHandleIMENotify( HWND hWnd, WPARAM
wParam )
static bool
ImplHandleGetObject(HWND hWnd, LPARAM lParam, WPARAM wParam, LRESULT & nRet)
{
- if (!Application::GetSettings().GetMiscSettings().GetEnableATToolSupport())
+ uno::Reference<accessibility::XMSAAService> xMSAA;
+ if (ImplSalYieldMutexTryToAcquire())
{
- // IA2 should be enabled automatically
- AllSettings aSettings = Application::GetSettings();
- MiscSettings aMisc = aSettings.GetMiscSettings();
- aMisc.SetEnableATToolSupport(true);
- // The above is enough, since aMisc changes the same shared
ImplMiscData as used in global
- // settings, so no need to call aSettings.SetMiscSettings and
Application::SetSettings
-
if
(!Application::GetSettings().GetMiscSettings().GetEnableATToolSupport())
- return false; // locked down somehow ?
- }
+ {
+ // IA2 should be enabled automatically
+ AllSettings aSettings = Application::GetSettings();
+ MiscSettings aMisc = aSettings.GetMiscSettings();
+ aMisc.SetEnableATToolSupport(true);
+ // The above is enough, since aMisc changes the same shared
ImplMiscData as used in global
+ // settings, so no need to call aSettings.SetMiscSettings and
Application::SetSettings
+
+ if
(!Application::GetSettings().GetMiscSettings().GetEnableATToolSupport())
+ return false; // locked down somehow ?
+ }
- ImplSVData* pSVData = ImplGetSVData();
+ ImplSVData* pSVData = ImplGetSVData();
- // Make sure to launch Accessibility only the following criteria are
satisfied
- // to avoid RFT interrupts regular accessibility processing
- if ( !pSVData->mxAccessBridge.is() )
- {
- if( !InitAccessBridge() )
- return false;
+ // Make sure to launch Accessibility only the following criteria are
satisfied
+ // to avoid RFT interrupts regular accessibility processing
+ if ( !pSVData->mxAccessBridge.is() )
+ {
+ if( !InitAccessBridge() )
+ return false;
+ }
+ xMSAA.set(pSVData->mxAccessBridge, uno::UNO_QUERY);
+ ImplSalYieldMutexRelease();
+ }
+ else
+ { // tdf#155794: access without locking: hopefully this should be fine
+ // as the bridge is typically inited in Desktop::Main() already and the
+ // WM_GETOBJECT is received only on the main thread and by the time in
+ // VCL shutdown when ImplSvData dies there should not be Windows any
+ // more that could receive messages.
+ xMSAA.set(ImplGetSVData()->mxAccessBridge, uno::UNO_QUERY);
}
- uno::Reference< accessibility::XMSAAService > xMSAA(
pSVData->mxAccessBridge, uno::UNO_QUERY );
if ( xMSAA.is() )
{
sal_Int32 lParam32 = static_cast<sal_Int32>(lParam);
@@ -5963,12 +5976,11 @@ static LRESULT CALLBACK SalFrameWndProc( HWND hWnd,
UINT nMsg, WPARAM wParam, LP
break;
case WM_GETOBJECT:
- ImplSalYieldMutexAcquireWithWait();
+ // tdf#155794: this must complete without taking SolarMutex
if ( ImplHandleGetObject( hWnd, lParam, wParam, nRet ) )
{
rDef = false;
}
- ImplSalYieldMutexRelease();
break;
case WM_APPCOMMAND:
