Avoid deadlock because of recursive lock
As seen in the following call stacks:
Main thread
sal3!osl_acquireMutex+0x49 [libre-office-git-repo\sal\osl\w32\mutex.cxx @ 65]
emboleobj!osl::Mutex::acquire+0x9 [libre-office-git-repo\include\osl\mutex.hxx @ 63]
emboleobj!osl::Guard<osl::Mutex>::{ctor}+0x9 [libre-office-git-repo\include\osl\mutex.hxx @ 144]
emboleobj!OleEmbeddedObject::getCurrentState+0x68 [libre-office-git-repo\embeddedobj\source\msole\oleembed.cxx @ 643]
sfxlo!SfxInPlaceClient::IsObjectUIActive+0x23 [libre-office-git-repo\sfx2\source\view\ipclient.cxx @ 847]
sfxlo!SfxViewShell::GetUIActiveClient+0x5b [libre-office-git-repo\sfx2\source\view\viewsh.cxx @ 2521]
sfxlo!SfxDispatcher::FindServer_+0x282 [libre-office-git-repo\sfx2\source\control\dispatch.cxx @ 1627]
sfxlo!SfxDispatcher::GetShellAndSlot_Impl+0x2f [libre-office-git-repo\sfx2\source\control\dispatch.cxx @ 703]
sfxlo!SfxDispatcher::QueryState+0x5b [libre-office-git-repo\sfx2\source\control\dispatch.cxx @ 1979]
swlo!SwView::CheckReadonlyState+0x64 [libre-office-git-repo\sw\source\uibase\uiview\view.cxx @ 627]
swlo!SwView::TimeoutHdl+0x76 [libre-office-git-repo\sw\source\uibase\uiview\view.cxx @ 608]
vcllo!Scheduler::CallbackTaskScheduling+0x130e [libre-office-git-repo\vcl\source\app\scheduler.cxx @ 511]
vclplug_winlo!WinSalTimer::ImplHandleElapsedTimer+0x2e [libre-office-git-repo\vcl\win\app\saltimer.cxx @ 167]
vclplug_winlo!ImplSalYield+0x14f [libre-office-git-repo\vcl\win\app\salinst.cxx @ 525]
vclplug_winlo!WinSalInstance::DoYield+0xad [libre-office-git-repo\vcl\win\app\salinst.cxx @ 581]
vcllo!ImplYield+0x367 [libre-office-git-repo\vcl\source\app\svapp.cxx @ 393]
vcllo!Application::Execute+0xfa [libre-office-git-repo\vcl\source\app\svapp.cxx @ 369]
sofficeapp!desktop::Desktop::Main+0x173f [libre-office-git-repo\desktop\source\app\app.cxx @ 1605]
vcllo!ImplSVMain+0xda [libre-office-git-repo\vcl\source\app\svmain.cxx @ 229]
sofficeapp!soffice_main+0xf3 [libre-office-git-repo\desktop\source\app\sofficemain.cxx @ 94]
Request thread
sal3!osl_acquireMutex+0x49 [libre-office-git-repo\sal\osl\w32\mutex.cxx @ 65]
vclplug_winlo!osl::Mutex::acquire+0xa [libre-office-git-repo\include\osl\mutex.hxx @ 63]
vclplug_winlo!SalYieldMutex::doAcquire+0x91 [libre-office-git-repo\vcl\win\app\salinst.cxx @ 148]
emboleobj!SolarMutexReleaser::{dtor}+0xc [libre-office-git-repo\include\vcl\svapp.hxx @ 1425]
emboleobj!OleComponent::GetExtent+0x102 [libre-office-git-repo\embeddedobj\source\msole\olecomponent.cxx @ 1134]
emboleobj!OleEmbeddedObject::getVisualAreaSize+0x23f [libre-office-git-repo\embeddedobj\source\msole\olevisual.cxx @ 227]
emboleobj!OleEmbeddedObject::saveCompleted+0x406 [libre-office-git-repo\embeddedobj\source\msole\olepersist.cxx @ 1603]
comphelper!comphelper::EmbeddedObjectContainer::StoreEmbeddedObject+0x49b [libre-office-git-repo\comphelper\source\container\embeddedobjectcontainer.cxx @ 501]
comphelper!comphelper::EmbeddedObjectContainer::InsertEmbeddedObject+0x6f [libre-office-git-repo\comphelper\source\container\embeddedobjectcontainer.cxx @ 518]
comphelper!comphelper::EmbeddedObjectContainer::RemoveEmbeddedObject+0x388 [libre-office-git-repo\comphelper\source\container\embeddedobjectcontainer.cxx @ 945]
comphelper!comphelper::EmbeddedObjectContainer::RemoveEmbeddedObject+0x46 [libre-office-git-repo\comphelper\source\container\embeddedobjectcontainer.cxx @ 831]
swlo!SwOLENode::SavePersistentData+0x20b [libre-office-git-repo\sw\source\core\ole\ndole.cxx @ 403]
swlo!SwNodes::ChgNode+0x411 [libre-office-git-repo\sw\source\core\docnode\nodes.cxx @ 214]
swlo!SwNodes::MoveNodes+0x18e6 [libre-office-git-repo\sw\source\core\docnode\nodes.cxx @ 868]
swlo!SwUndoSaveContent::MoveToUndoNds+0x1c1 [libre-office-git-repo\sw\source\core\undo\undobj.cxx @ 784]
swlo!SwUndoSaveSection::SaveSection+0x559 [libre-office-git-repo\sw\source\core\undo\undobj.cxx @ 1325]
swlo!SwUndoSaveSection::SaveSection+0x55 [libre-office-git-repo\sw\source\core\undo\undobj.cxx @ 1278]
swlo!SwUndoFlyBase::DelFly+0x127 [libre-office-git-repo\sw\source\core\undo\undobj1.cxx @ 232]
swlo!SwUndoDelLayFormat::SwUndoDelLayFormat+0x64 [libre-office-git-repo\sw\source\core\undo\undobj1.cxx @ 429]
swlo!std::make_unique+0x22 [C:\Program Files (x86)\Microsoft Visual Studio\2019\BuildTools\VC\Tools\MSVC\14.29.30133\include\memory @ 3382]
swlo!sw::DocumentLayoutManager::DelLayoutFormat+0x283 [libre-office-git-repo\sw\source\core\doc\DocumentLayoutManager.cxx @ 242]
swlo!SwTextNode::DestroyAttr+0x82 [libre-office-git-repo\sw\source\core\txtnode\thints.cxx @ 1204]
swlo!SwTextNode::EraseText+0x18f [libre-office-git-repo\sw\source\core\txtnode\ndtxt.cxx @ 2813]
swlo!SwTextNode::DeleteAttributes+0x115 [libre-office-git-repo\sw\source\core\txtnode\thints.cxx @ 1863]
swlo!SwXFrame::dispose+0xdb [libre-office-git-repo\sw\source\core\unocore\unoframe.cxx @ 2692]
mscx_uno!`anonymous namespace'::cpp_call+0x710 [libre-office-git-repo\bridges\source\cpp_uno\msvc_win32_x86-64\uno2cpp.cxx @ 214]
mscx_uno!unoInterfaceProxyDispatch+0x2fa [libre-office-git-repo\bridges\source\cpp_uno\msvc_win32_x86-64\uno2cpp.cxx @ 430]
binaryurplo!binaryurp::IncomingRequest::execute_throw+0x635 [libre-office-git-repo\binaryurp\source\incomingrequest.cxx @ 239]
binaryurplo!binaryurp::IncomingRequest::execute+0xbf [libre-office-git-repo\binaryurp\source\incomingrequest.cxx @ 79]
binaryurplo!request+0x1c [libre-office-git-repo\binaryurp\source\reader.cxx @ 84]
cppu3!cppu_threadpool::JobQueue::enter+0x21e [libre-office-git-repo\cppu\source\threadpool\jobqueue.cxx @ 101]
cppu3!cppu_threadpool::ORequestThread::run+0xa0 [libre-office-git-repo\cppu\source\threadpool\thread.cxx @ 169]
As seen, the request thread has OleEmbeddedObject::saveCompleted
and OleEmbeddedObject::getVisualAreaSize on the stack; both acquire
OleEmbeddedObject's mutex; but only getVisualAreaSize releases it
for the call of OleComponent::GetExtent. The latter releases solar
mutex, at which time, the main thread (locking the solar mutex)
attempts to call OleEmbeddedObject::getCurrentState, which needs
the object's mutex, which is still locked in the request thread.
Avoid this, by passing the lock object to the implementation of
getVisualAreaSize from saveCompleted.
Change-Id: Ie44a20a8c89000e0e951e024c2bfde93cf2cc3f6
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/174891
Tested-by: Jenkins
Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>
This commit is contained in:
Mike Kaganski
@@ -258,7 +258,8 @@ protected:
|
||||
/// @throws css::uno::Exception
|
||||
void InsertVisualCache_Impl(
|
||||
const css::uno::Reference< css::io::XStream >& xTargetStream,
|
||||
const css::uno::Reference< css::io::XStream >& xCachedVisualRepresentation );
|
||||
const css::uno::Reference< css::io::XStream >& xCachedVisualRepresentation,
|
||||
osl::ResettableMutexGuard& rGuard);
|
||||
|
||||
/// @throws css::uno::Exception
|
||||
void RemoveVisualCache_Impl( const css::uno::Reference< css::io::XStream >& xTargetStream );
|
||||
@@ -453,6 +454,9 @@ public:
|
||||
OUString SAL_CALL getImplementationName() override;
|
||||
sal_Bool SAL_CALL supportsService( const OUString& ServiceName ) override;
|
||||
css::uno::Sequence< OUString > SAL_CALL getSupportedServiceNames() override;
|
||||
|
||||
private:
|
||||
css::awt::Size getVisualAreaSize_impl(sal_Int64 nAspect, osl::ResettableMutexGuard& guard);
|
||||
};
|
||||
|
||||
namespace
|
||||
|
||||
@@ -353,7 +353,8 @@ uno::Reference< io::XStream > OleEmbeddedObject::TryToGetAcceptableFormat_Impl(
|
||||
|
||||
|
||||
void OleEmbeddedObject::InsertVisualCache_Impl( const uno::Reference< io::XStream >& xTargetStream,
|
||||
const uno::Reference< io::XStream >& xCachedVisualRepresentation )
|
||||
const uno::Reference< io::XStream >& xCachedVisualRepresentation,
|
||||
osl::ResettableMutexGuard& rGuard )
|
||||
{
|
||||
OSL_ENSURE( xTargetStream.is() && xCachedVisualRepresentation.is(), "Invalid arguments!" );
|
||||
|
||||
@@ -433,7 +434,7 @@ void OleEmbeddedObject::InsertVisualCache_Impl( const uno::Reference< io::XStrea
|
||||
xTempOutStream->writeBytes( aData );
|
||||
|
||||
// get the size
|
||||
awt::Size aSize = getVisualAreaSize( embed::Aspects::MSOLE_CONTENT );
|
||||
awt::Size aSize = getVisualAreaSize_impl(embed::Aspects::MSOLE_CONTENT, rGuard);
|
||||
sal_Int32 nIndex = 0;
|
||||
|
||||
// write width
|
||||
@@ -1223,7 +1224,7 @@ void OleEmbeddedObject::StoreToLocation_Impl(
|
||||
}
|
||||
}
|
||||
|
||||
InsertVisualCache_Impl( xTargetStream, xCachedVisualRepresentation );
|
||||
InsertVisualCache_Impl(xTargetStream, xCachedVisualRepresentation, rGuard);
|
||||
}
|
||||
else
|
||||
{
|
||||
@@ -1600,7 +1601,7 @@ void SAL_CALL OleEmbeddedObject::saveCompleted( sal_Bool bUseNew )
|
||||
{
|
||||
// the call will cache the size in case of success
|
||||
// probably it might need to be done earlier, while the object is in active state
|
||||
getVisualAreaSize( embed::Aspects::MSOLE_CONTENT );
|
||||
getVisualAreaSize_impl(embed::Aspects::MSOLE_CONTENT, aGuard);
|
||||
}
|
||||
catch( const uno::Exception& )
|
||||
{}
|
||||
@@ -1750,7 +1751,7 @@ void SAL_CALL OleEmbeddedObject::storeOwn()
|
||||
{
|
||||
// the m_xCachedVisualRepresentation must be set or it should be already stored
|
||||
if ( m_xCachedVisualRepresentation.is() )
|
||||
InsertVisualCache_Impl( m_xObjectStream, m_xCachedVisualRepresentation );
|
||||
InsertVisualCache_Impl(m_xObjectStream, m_xCachedVisualRepresentation, aGuard);
|
||||
else
|
||||
{
|
||||
m_xCachedVisualRepresentation = TryToRetrieveCachedVisualRepresentation_Impl( m_xObjectStream, aGuard );
|
||||
@@ -1775,7 +1776,7 @@ void SAL_CALL OleEmbeddedObject::storeOwn()
|
||||
{
|
||||
// the call will cache the size in case of success
|
||||
// probably it might need to be done earlier, while the object is in active state
|
||||
getVisualAreaSize( embed::Aspects::MSOLE_CONTENT );
|
||||
getVisualAreaSize_impl(embed::Aspects::MSOLE_CONTENT, aGuard);
|
||||
}
|
||||
catch( const uno::Exception& )
|
||||
{}
|
||||
|
||||
@@ -155,18 +155,19 @@ void SAL_CALL OleEmbeddedObject::setVisualAreaSize( sal_Int64 nAspect, const awt
|
||||
m_nCachedAspect = nAspect;
|
||||
}
|
||||
|
||||
awt::Size SAL_CALL OleEmbeddedObject::getVisualAreaSize( sal_Int64 nAspect )
|
||||
awt::Size OleEmbeddedObject::getVisualAreaSize_impl(sal_Int64 nAspect,
|
||||
osl::ResettableMutexGuard& guard)
|
||||
{
|
||||
// begin wrapping related part ====================
|
||||
uno::Reference< embed::XEmbeddedObject > xWrappedObject = m_xWrappedObject;
|
||||
if ( xWrappedObject.is() )
|
||||
{
|
||||
osl::ResettableMutexGuardScopedReleaser area(guard);
|
||||
// the object was converted to OOo embedded object, the current implementation is now only a wrapper
|
||||
return xWrappedObject->getVisualAreaSize( nAspect );
|
||||
}
|
||||
// end wrapping related part ====================
|
||||
|
||||
::osl::ResettableMutexGuard aGuard( m_aMutex );
|
||||
if ( m_bDisposed )
|
||||
throw lang::DisposedException(); // TODO
|
||||
|
||||
@@ -197,7 +198,9 @@ awt::Size SAL_CALL OleEmbeddedObject::getVisualAreaSize( sal_Int64 nAspect )
|
||||
{
|
||||
// there is no internal cache
|
||||
awt::Size aSize;
|
||||
aGuard.clear();
|
||||
|
||||
{ // => unguarded
|
||||
osl::ResettableMutexGuardScopedReleaser area(guard);
|
||||
|
||||
bool bBackToLoaded = false;
|
||||
|
||||
@@ -277,7 +280,7 @@ awt::Size SAL_CALL OleEmbeddedObject::getVisualAreaSize( sal_Int64 nAspect )
|
||||
"No size available!",
|
||||
static_cast< ::cppu::OWeakObject* >(this) );
|
||||
|
||||
aGuard.reset();
|
||||
} // <= unguarded
|
||||
|
||||
m_aCachedSize = aSize;
|
||||
m_nCachedAspect = nAspect;
|
||||
@@ -314,6 +317,12 @@ awt::Size SAL_CALL OleEmbeddedObject::getVisualAreaSize( sal_Int64 nAspect )
|
||||
return aResult;
|
||||
}
|
||||
|
||||
awt::Size SAL_CALL OleEmbeddedObject::getVisualAreaSize( sal_Int64 nAspect )
|
||||
{
|
||||
osl::ResettableMutexGuard aGuard(m_aMutex);
|
||||
return getVisualAreaSize_impl(nAspect, aGuard);
|
||||
}
|
||||
|
||||
embed::VisualRepresentation SAL_CALL OleEmbeddedObject::getPreferredVisualRepresentation( sal_Int64 nAspect )
|
||||
{
|
||||
// begin wrapping related part ====================
|
||||
|
||||
Reference in New Issue
Block a user