vcl/inc/win/salinst.h | 1 vcl/win/app/salinst.cxx | 128 ++++++++---------------------------------------- 2 files changed, 23 insertions(+), 106 deletions(-)
New commits: commit c5174cb7597d2c0ddbc356b3456a92e96707b71a Author: Mike Kaganski <[email protected]> AuthorDate: Thu Mar 5 09:29:41 2026 +0100 Commit: Mike Kaganski <[email protected]> CommitDate: Thu Mar 5 11:28:00 2026 +0100 tdf#170205: drop NoYieldLock mode Added in commit 4baec725e0dc0713f0d47003e9b10bc3b62f56ff (WIN run main thread redirects ignoring SolarMutex, 2017-09-26), it tried to handle SendMessage calls without releasing SolarMutex in the calling code. The idea seemed to be, that the calling thread keeps holding SolarMutex while waiting for the main thread, and main thread has this mode that turns all calls to acquire/release/test solar mutex to noop (which was safe, since the thread holding the mutex is known to be blocked). Unfortunately, that doesn't work. The many hangs discovered since then (e.g., commit 1e2e51607a163021ef1fb1fb0d217266bd448173 - Try to use CoWaitForMultipleHandles magic to handle COM message loop, 2025-01-10) required to unlock solar mutex when calling SendMessage (implemented in commit 23afeaedf4d4a03943338fc39ae41f5c423e5997 - tdf#168431: Release solar mutex when sending window message to main thread, 2025-09-09). It made it possibe, that SolarMutex::doRelease called from non-main thread got false from IsCurrentThread, because the NoYieldLock mode was active, and SalYieldMutex::IsCurrentThread only checked that current thread is main; that caused abort(). This change removes this special mode. The intention is, that every call to SendMessage must release solar mutex, and allow main thread to lock it. Hope this is a reasonable approach. Change-Id: I838be4d366b24727cd93610d9cacfbe14992923e Reviewed-on: https://gerrit.libreoffice.org/c/core/+/201007 Reviewed-by: Noel Grandin <[email protected]> Reviewed-by: Mike Kaganski <[email protected]> Tested-by: Jenkins diff --git a/vcl/inc/win/salinst.h b/vcl/inc/win/salinst.h index 0342b951e204..8f2b64e79304 100644 --- a/vcl/inc/win/salinst.h +++ b/vcl/inc/win/salinst.h @@ -38,7 +38,6 @@ public: HWND mhComWnd; osl::Condition maWaitingYieldCond; - unsigned m_nNoYieldLock; public: WinSalInstance(SalData* pSalData); diff --git a/vcl/win/app/salinst.cxx b/vcl/win/app/salinst.cxx index 4df35127560f..e84ca94bfd02 100644 --- a/vcl/win/app/salinst.cxx +++ b/vcl/win/app/salinst.cxx @@ -86,9 +86,6 @@ protected: public: explicit SalYieldMutex(); - - virtual bool IsCurrentThread() const override; - virtual bool tryToAcquire() override; }; SalYieldMutex::SalYieldMutex() @@ -116,8 +113,6 @@ void SalYieldMutex::doAcquire( sal_uInt32 nLockCount ) WinSalInstance* pInst = GetWinSalInstance(); if ( pInst && pInst->IsMainThread() ) { - if ( pInst->m_nNoYieldLock ) - return; // tdf#96887 If this is the main thread, then we must wait for two things: // - the yield mutex being unlocked // - SendMessage() being triggered @@ -153,10 +148,6 @@ void SalYieldMutex::doAcquire( sal_uInt32 nLockCount ) sal_uInt32 SalYieldMutex::doRelease( const bool bUnlockAll ) { - WinSalInstance* pInst = GetWinSalInstance(); - if ( pInst && pInst->m_nNoYieldLock && pInst->IsMainThread() ) - return 1; - sal_uInt32 nCount = comphelper::SolarMutex::doRelease( bUnlockAll ); // wake up ImplSalYieldMutexAcquireWithWait() after release if ( 0 == m_nCount ) @@ -164,20 +155,6 @@ sal_uInt32 SalYieldMutex::doRelease( const bool bUnlockAll ) return nCount; } -bool SalYieldMutex::tryToAcquire() -{ - WinSalInstance* pInst = GetWinSalInstance(); - if ( pInst ) - { - if ( pInst->m_nNoYieldLock && pInst->IsMainThread() ) - return true; - else - return comphelper::SolarMutex::tryToAcquire(); - } - else - return false; -} - void ImplSalYieldMutexAcquireWithWait( sal_uInt32 nCount ) { WinSalInstance* pInst = GetWinSalInstance(); @@ -201,14 +178,6 @@ void ImplSalYieldMutexRelease() } } -bool SalYieldMutex::IsCurrentThread() const -{ - if (!GetWinSalInstance()->m_nNoYieldLock) - return SolarMutex::IsCurrentThread(); - else - return GetWinSalInstance()->IsMainThread(); -} - void SalData::initKeyCodeMap() { auto initKey = [this](wchar_t ch, sal_uInt16 key) @@ -394,7 +363,6 @@ WinSalInstance::WinSalInstance(SalData* pSalData) : SalInstance(std::make_unique<SalYieldMutex>(), pSalData) , mhInst( nullptr ) , mhComWnd( nullptr ) - , m_nNoYieldLock( 0 ) { ImplSVData* pSVData = ImplGetSVData(); pSVData->maAppData.mxToolkitName = OUString("win"); @@ -435,12 +403,6 @@ bool ImplSalYield(const bool bWait, const bool bHandleAllCurrentEvents) // used to abort further message processing on tick count wraps static sal_uInt32 nLastTicks = 0; - // we should never yield in m_nNoYieldLock mode! - const bool bNoYieldLock = (GetWinSalInstance()->m_nNoYieldLock > 0); - assert(!bNoYieldLock); - if (bNoYieldLock) - return false; - MSG aMsg; bool bWasMsg = false, bWasTimeoutMsg = false; WinSalTimer* pTimer = static_cast<WinSalTimer*>(ImplGetSVData()->maSchedCtx.mpSalTimer); @@ -537,26 +499,6 @@ bool WinSalInstance::DoYield(bool bWait, bool bHandleAllCurrentEvents) return bDidWork; } -namespace -{ -struct NoYieldLockGuard -{ - NoYieldLockGuard() - : counter(InSendMessage() ? GetWinSalInstance()->m_nNoYieldLock : dummy()) - { - ++counter; - } - ~NoYieldLockGuard() { --counter; } - static decltype(WinSalInstance::m_nNoYieldLock)& dummy() - { - DBG_TESTSOLARMUTEX(); // called when !InSendMessage() - static decltype(WinSalInstance::m_nNoYieldLock) n = 0; - return n; - } - decltype(WinSalInstance::m_nNoYieldLock)& counter; -}; -} - LRESULT CALLBACK SalComWndProc( HWND, UINT nMsg, WPARAM wParam, LPARAM lParam, bool& rDef ) { LRESULT nRet = 0; @@ -597,33 +539,21 @@ LRESULT CALLBACK SalComWndProc( HWND, UINT nMsg, WPARAM wParam, LPARAM lParam, b pTimer->ImplStop(); break; - case (SAL_MSG_CREATEFRAME): - { - NoYieldLockGuard g; - nRet = reinterpret_cast<LRESULT>( - ImplSalCreateFrame(GetWinSalInstance(), reinterpret_cast<HWND>(lParam), - static_cast<SalFrameStyleFlags>(wParam))); - } + case SAL_MSG_CREATEFRAME: + nRet = reinterpret_cast<LRESULT>( + ImplSalCreateFrame(GetWinSalInstance(), reinterpret_cast<HWND>(lParam), + static_cast<SalFrameStyleFlags>(wParam))); break; - case (SAL_MSG_RECREATEHWND): - { - NoYieldLockGuard g; - nRet = reinterpret_cast<LRESULT>(ImplSalReCreateHWND( - reinterpret_cast<HWND>(wParam), reinterpret_cast<HWND>(lParam), false)); - } + case SAL_MSG_RECREATEHWND: + nRet = reinterpret_cast<LRESULT>(ImplSalReCreateHWND( + reinterpret_cast<HWND>(wParam), reinterpret_cast<HWND>(lParam), false)); break; - case (SAL_MSG_RECREATECHILDHWND): - { - NoYieldLockGuard g; - nRet = reinterpret_cast<LRESULT>(ImplSalReCreateHWND( - reinterpret_cast<HWND>(wParam), reinterpret_cast<HWND>(lParam), true)); - } + case SAL_MSG_RECREATECHILDHWND: + nRet = reinterpret_cast<LRESULT>(ImplSalReCreateHWND( + reinterpret_cast<HWND>(wParam), reinterpret_cast<HWND>(lParam), true)); break; - case (SAL_MSG_DESTROYFRAME): - { - NoYieldLockGuard g; - delete reinterpret_cast<SalFrame*>(lParam); - } + case SAL_MSG_DESTROYFRAME: + delete reinterpret_cast<SalFrame*>(lParam); break; case SAL_MSG_DESTROYHWND: @@ -638,31 +568,19 @@ LRESULT CALLBACK SalComWndProc( HWND, UINT nMsg, WPARAM wParam, LPARAM lParam, b } break; - case (SAL_MSG_CREATEOBJECT): - { - NoYieldLockGuard g; - nRet = reinterpret_cast<LRESULT>(ImplSalCreateObject( - GetWinSalInstance(), reinterpret_cast<WinSalFrame*>(lParam))); - } + case SAL_MSG_CREATEOBJECT: + nRet = reinterpret_cast<LRESULT>( + ImplSalCreateObject(GetWinSalInstance(), reinterpret_cast<WinSalFrame*>(lParam))); break; - case (SAL_MSG_DESTROYOBJECT): - { - NoYieldLockGuard g; - delete reinterpret_cast<SalObject*>(lParam); - } + case SAL_MSG_DESTROYOBJECT: + delete reinterpret_cast<SalObject*>(lParam); break; - case (SAL_MSG_GETCACHEDDC): - { - NoYieldLockGuard g; - nRet = reinterpret_cast<LRESULT>( - GetDCEx(reinterpret_cast<HWND>(wParam), nullptr, DCX_CACHE)); - } + case SAL_MSG_GETCACHEDDC: + nRet = reinterpret_cast<LRESULT>( + GetDCEx(reinterpret_cast<HWND>(wParam), nullptr, DCX_CACHE)); break; - case (SAL_MSG_RELEASEDC): - { - NoYieldLockGuard g; - ReleaseDC(reinterpret_cast<HWND>(wParam), reinterpret_cast<HDC>(lParam)); - } + case SAL_MSG_RELEASEDC: + ReleaseDC(reinterpret_cast<HWND>(wParam), reinterpret_cast<HDC>(lParam)); break; case SAL_MSG_TIMER_CALLBACK: @@ -761,7 +679,7 @@ bool WinSalInstance::AnyInput( VclInputFlags nType ) LRESULT WinSalInstance::SendWndMessage_impl(HWND hWnd, UINT Msg, WPARAM wParam, LPARAM lParam) const { std::optional<SolarMutexReleaser> oReleaser; - if (!IsMainThread()) + if (GetCurrentThreadId() != GetWindowThreadProcessId(hWnd, nullptr)) oReleaser.emplace(); return SendMessageW(hWnd, Msg, wParam, lParam); }
