embeddedobj/source/inc/oleembobj.hxx | 4 -
embeddedobj/source/msole/olecomponent.cxx | 3
embeddedobj/source/msole/oleembed.cxx | 71 ++++++++++-------------
embeddedobj/source/msole/olemisc.cxx | 92 +++++++++++++++---------------
embeddedobj/source/msole/olepersist.cxx | 33 ++++------
5 files changed, 98 insertions(+), 105 deletions(-)
New commits:
commit 19d71a2926e35d57dbbfa0468ef5fba4ad25e525
Author: Mike Kaganski <mike.kagan...@collabora.com>
AuthorDate: Wed Sep 4 14:38:23 2024 +0500
Commit: Mike Kaganski <mike.kagan...@collabora.com>
CommitDate: Mon Sep 9 11:57:30 2024 +0500
Avoid deadlock
Seen when running an external Java code, with these call stacks:
Main thread:
win32u.dll!NtUserMsgWaitForMultipleObjectsEx()
sal3.dll!osl_waitCondition(void * Condition, const TimeValue * pTimeout)
[Inline Frame] vclplug_winlo.dll!osl::Condition::wait(const TimeValue *)
vclplug_winlo.dll!SalYieldMutex::doAcquire(unsigned long nLockCount)
[Inline Frame] emboleobj.dll!SolarMutexReleaser::{dtor}()
emboleobj.dll!OleComponent::getTransferData(const
com::sun::star::datatransfer::DataFlavor & aFlavor)
emboleobj.dll!OleEmbeddedObject::getPreferredVisualRepresentation(__int64
nAspect)
comphelper.dll!comphelper::EmbeddedObjectContainer::GetGraphicReplacementStream(__int64
nViewAspect, const
com::sun::star::uno::Reference<com::sun::star::embed::XEmbeddedObject> & xObj,
rtl::OUString * pMediaType)
[Inline Frame]
svtlo.dll!svt::EmbeddedObjectRef::GetGraphicReplacementStream(__int64)
svtlo.dll!svt::EmbeddedObjectRef::GetGraphicStream(bool bUpdate)
svtlo.dll!svt::EmbeddedObjectRef::GetReplacement(bool bUpdate)
svtlo.dll!svt::EmbeddedObjectRef::UpdateReplacement(bool bUpdateOle)
swlo.dll!SwWrtShell::CalcAndSetScale(svt::EmbeddedObjectRef & xObj, const
SwRect * pFlyPrtRect, const SwRect * pFlyFrameRect, const bool
bNoTextFramePrtAreaChanged)
swlo.dll!SwContentNotify::ImplDestroy()
swlo.dll!SwContentNotify::~SwContentNotify()
swlo.dll!SwNoTextFrame::MakeAll(OutputDevice * pRenderContext)
swlo.dll!SwFrame::OptPrepareMake()
[Inline Frame] swlo.dll!SwFrame::OptCalc()
swlo.dll!SwLayAction::FormatContent_(const SwContentFrame * pContent,
const SwPageFrame * pPage)
swlo.dll!SwLayAction::FormatFlyContent(const SwFlyFrame * pFly)
swlo.dll!SwObjectFormatter::FormatObj_(SwAnchoredObject & _rAnchoredObj)
swlo.dll!SwObjectFormatterTextFrame::DoFormatObj(SwAnchoredObject &
_rAnchoredObj, const bool _bCheckForMovedFwd)
swlo.dll!SwObjectFormatter::FormatObjsAtFrame_(SwTextFrame *
_pMasterTextFrame)
swlo.dll!SwObjectFormatterTextFrame::DoFormatObjs()
swlo.dll!SwObjectFormatter::FormatObjsAtFrame(SwFrame & _rAnchorFrame,
const SwPageFrame & _rPageFrame, SwLayAction * _pLayAction)
swlo.dll!SwLayAction::FormatContent(SwPageFrame * pPage)
swlo.dll!SwLayAction::InternalAction(OutputDevice * pRenderContext)
[Inline Frame] swlo.dll!SwLayAction::Action(OutputDevice * pRenderContext)
swlo.dll!SwLayIdle::SwLayIdle(SwRootFrame * pRt, SwViewShellImp * pI)
swlo.dll!SwViewShell::LayoutIdle()
swlo.dll!sw::DocumentTimerManager::DoIdleJobs(Timer * __formal)
vcllo.dll!Scheduler::CallbackTaskScheduling()
vclplug_winlo.dll!WinSalTimer::ImplHandleElapsedTimer()
vclplug_winlo.dll!ImplSalYield(bool bWait, bool bHandleAllCurrentEvents)
vclplug_winlo.dll!WinSalInstance::DoYield(bool bWait, bool
bHandleAllCurrentEvents)
vcllo.dll!ImplYield(bool i_bWait, bool i_bAllEvents)
vcllo.dll!Application::Execute()
sofficeapp.dll!desktop::Desktop::Main()
vcllo.dll!ImplSVMain()
sofficeapp.dll!soffice_main()
[Inline Frame] soffice.bin!sal_main()
soffice.bin!main(int argc, char * * argv)
[Inline Frame] soffice.bin!invoke_main()
soffice.bin!__scrt_common_main_seh()
kernel32.dll!BaseThreadInitThunk()
ntdll.dll!RtlUserThreadStart()
Request thread:
ntdll.dll!NtWaitForAlertByThreadId()
ntdll.dll!RtlpWaitOnCriticalSection()
ntdll.dll!RtlpEnterCriticalSectionContended()
ntdll.dll!RtlEnterCriticalSection()
sal3.dll!osl_acquireMutex(_oslMutexImpl * Mutex)
[Inline Frame] emboleobj.dll!osl::Mutex::acquire()
[Inline Frame] emboleobj.dll!osl::Guard<osl::Mutex>::{ctor}(osl::Mutex &)
emboleobj.dll!OleComponent::addModifyListener(const
com::sun::star::uno::Reference<com::sun::star::util::XModifyListener> &
xListener)
svtlo.dll!svt::`anonymous
namespace'::EmbedEventListener_Impl::Create(svt::EmbeddedObjectRef * p)
svtlo.dll!svt::EmbeddedObjectRef::EmbeddedObjectRef(const
com::sun::star::uno::Reference<com::sun::star::embed::XEmbeddedObject> & xObj,
__int64 nAspect)
swlo.dll!SwXFrame::getPropertyValue(const rtl::OUString & rPropertyName)
mscx_uno.dll!`anonymous
namespace'::cpp_call(bridges::cpp_uno::shared::UnoInterfaceProxy * pThis,
bridges::cpp_uno::shared::VtableSlot aVtableSlot,
_typelib_TypeDescriptionReference * pReturnTypeRef, long nParams,
_typelib_MethodParameter * pParams, void * pUnoReturn, void * * pUnoArgs,
_uno_Any * * ppUnoExc)
mscx_uno.dll!unoInterfaceProxyDispatch(_uno_Interface * pUnoI, const
_typelib_TypeDescription * pMemberTD, void * pReturn, void * * pArgs, _uno_Any
* * ppException)
binaryurplo.dll!binaryurp::IncomingRequest::execute_throw(binaryurp::BinaryAny
* returnValue,
std::vector<binaryurp::BinaryAny,std::allocator<binaryurp::BinaryAny>> *
outArguments)
binaryurplo.dll!binaryurp::IncomingRequest::execute()
binaryurplo.dll!request(void * pThreadSpecificData)
cppu3.dll!cppu_threadpool::JobQueue::enter(const void * nDisposeId, bool
bReturnWhenNoJob)
cppu3.dll!cppu_threadpool::ORequestThread::run()
cppu3.dll!threadFunc(void * param)
sal3.dll!oslWorkerWrapperFunction(void * pData)
ucrtbase.dll!thread_start<unsigned int (__cdecl*)(void *),1>()
kernel32.dll!BaseThreadInitThunk()
ntdll.dll!RtlUserThreadStart()
Change-Id: I54d3fd91d8bbf4f52eee2f498c91e06ebc82d836
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/172864
Reviewed-by: Mike Kaganski <mike.kagan...@collabora.com>
Tested-by: Jenkins
diff --git a/embeddedobj/source/msole/olecomponent.cxx
b/embeddedobj/source/msole/olecomponent.cxx
index 6038ef3551d6..4ccd7aa3a9e2 100644
--- a/embeddedobj/source/msole/olecomponent.cxx
+++ b/embeddedobj/source/msole/olecomponent.cxx
@@ -1541,7 +1541,7 @@ void SAL_CALL OleComponent::removeCloseListener( const
uno::Reference< util::XCl
uno::Any SAL_CALL OleComponent::getTransferData( const
datatransfer::DataFlavor& aFlavor )
{
- ::osl::MutexGuard aGuard( m_aMutex );
+ ::osl::ResettableMutexGuard aGuard( m_aMutex );
if ( m_bDisposed )
throw lang::DisposedException(); // TODO
@@ -1585,6 +1585,7 @@ uno::Any SAL_CALL OleComponent::getTransferData( const
datatransfer::DataFlavor&
HRESULT hr;
{
+ osl::ResettableMutexGuardScopedReleaser
own_releaser(aGuard);
SolarMutexReleaser releaser;
hr = pDataObject->GetData(&aFormat, &aMedium);
}
commit 8f3ada051928dd9e6602d05cd84b172ee9f39ced
Author: Mike Kaganski <mike.kagan...@collabora.com>
AuthorDate: Thu Aug 22 12:44:50 2024 +0500
Commit: Mike Kaganski <mike.kagan...@collabora.com>
CommitDate: Mon Sep 9 11:57:30 2024 +0500
Make OleEmbeddedObject locking stricter
This is an attempt to reduce areas that execute with mutex unlocked
because the called code may deadlock. Also, this copies objects on
stack before unlocking, using lambdas' own variables.
I hope that it somewhat improves reliability. OTOH, this runs more
code with lock active, has a potential of new deadlocks, so risky.
Change-Id: I558b7f5f18f63622fed3a9c3978327d0d76fe90d
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/172237
Tested-by: Jenkins
Reviewed-by: Mike Kaganski <mike.kagan...@collabora.com>
diff --git a/embeddedobj/source/inc/oleembobj.hxx
b/embeddedobj/source/inc/oleembobj.hxx
index e513c57085ce..0e18a168f00e 100644
--- a/embeddedobj/source/inc/oleembobj.hxx
+++ b/embeddedobj/source/inc/oleembobj.hxx
@@ -219,9 +219,9 @@ protected:
#ifdef _WIN32
void SwitchComponentToRunningState_Impl(osl::ResettableMutexGuard& guard);
#endif
- void MakeEventListenerNotification_Impl( const OUString& aEventName );
+ void MakeEventListenerNotification_Impl( const OUString& aEventName,
osl::ResettableMutexGuard& guard );
#ifdef _WIN32
- void StateChangeNotification_Impl( bool bBeforeChange, sal_Int32
nOldState, sal_Int32 nNewState );
+ void StateChangeNotification_Impl( bool bBeforeChange, sal_Int32
nOldState, sal_Int32 nNewState, osl::ResettableMutexGuard& guard );
css::uno::Reference< css::io::XOutputStream > GetStreamForSaving();
diff --git a/embeddedobj/source/msole/oleembed.cxx
b/embeddedobj/source/msole/oleembed.cxx
index 9a5d41abfabc..af0c74a5af6a 100644
--- a/embeddedobj/source/msole/oleembed.cxx
+++ b/embeddedobj/source/msole/oleembed.cxx
@@ -478,9 +478,7 @@ void SAL_CALL OleEmbeddedObject::changeState( sal_Int32
nNewState )
// will behave after activation
sal_Int32 nOldState = m_nObjectState;
- aGuard.clear();
- StateChangeNotification_Impl( true, nOldState, nNewState );
- aGuard.reset();
+ StateChangeNotification_Impl( true, nOldState, nNewState, aGuard );
try
{
@@ -495,14 +493,12 @@ void SAL_CALL OleEmbeddedObject::changeState( sal_Int32
nNewState )
// the loaded state must be set before, because of
notifications!
m_nObjectState = nNewState;
- aGuard.clear();
{
VerbExecutionControllerGuard aVerbGuard(
m_aVerbExecutionController );
- m_pOleComponent->CloseObject();
+ ExecUnlocked([p = m_pOleComponent] { p->CloseObject(); },
aGuard);
}
- StateChangeNotification_Impl( false, nOldState, m_nObjectState
);
- aGuard.reset();
+ StateChangeNotification_Impl( false, nOldState,
m_nObjectState, aGuard );
}
else if ( nNewState == embed::EmbedStates::RUNNING || nNewState ==
embed::EmbedStates::ACTIVE )
{
@@ -517,19 +513,17 @@ void SAL_CALL OleEmbeddedObject::changeState( sal_Int32
nNewState )
SwitchComponentToRunningState_Impl(aGuard);
m_nObjectState = embed::EmbedStates::RUNNING;
- aGuard.clear();
- StateChangeNotification_Impl( false, nOldState,
m_nObjectState );
- aGuard.reset();
+ StateChangeNotification_Impl( false, nOldState,
m_nObjectState, aGuard );
if ( m_pOleComponent && m_bHasSizeToSet )
{
- aGuard.clear();
try {
- m_pOleComponent->SetExtent( m_aSizeToSet,
m_nAspectToSet );
+ ExecUnlocked([p = m_pOleComponent, s =
m_aSizeToSet,
+ a = m_nAspectToSet]() {
p->SetExtent(s, a); },
+ aGuard);
m_bHasSizeToSet = false;
}
catch( const uno::Exception& ) {}
- aGuard.reset();
}
if ( m_nObjectState == nNewState )
@@ -541,30 +535,33 @@ void SAL_CALL OleEmbeddedObject::changeState( sal_Int32
nNewState )
if ( m_nObjectState == embed::EmbedStates::RUNNING &&
nNewState == embed::EmbedStates::ACTIVE )
{
// execute OPEN verb, if object does not reach active
state it is an object's problem
- aGuard.clear();
- m_pOleComponent->ExecuteVerb(
embed::EmbedVerbs::MS_OLEVERB_OPEN );
- aGuard.reset();
+ ExecUnlocked([p = m_pOleComponent]()
+ {
p->ExecuteVerb(embed::EmbedVerbs::MS_OLEVERB_OPEN); },
+ aGuard);
// some objects do not allow to set the size even in
running state
if ( m_pOleComponent && m_bHasSizeToSet )
{
- aGuard.clear();
try {
- m_pOleComponent->SetExtent( m_aSizeToSet,
m_nAspectToSet );
+ ExecUnlocked([p = m_pOleComponent, s =
m_aSizeToSet,
+ a = m_nAspectToSet]() {
p->SetExtent(s, a); },
+ aGuard);
m_bHasSizeToSet = false;
}
catch( uno::Exception& ) {}
- aGuard.reset();
}
m_nObjectState = nNewState;
}
else if ( m_nObjectState == embed::EmbedStates::ACTIVE &&
nNewState == embed::EmbedStates::RUNNING )
{
- aGuard.clear();
- m_pOleComponent->CloseObject();
- m_pOleComponent->RunObject(); // Should not fail, the
object already was active
- aGuard.reset();
+ ExecUnlocked(
+ [p = m_pOleComponent]()
+ {
+ p->CloseObject();
+ p->RunObject(); // Should not fail, the object
already was active
+ },
+ aGuard);
m_nObjectState = nNewState;
}
else
@@ -577,8 +574,7 @@ void SAL_CALL OleEmbeddedObject::changeState( sal_Int32
nNewState )
}
catch( uno::Exception& )
{
- aGuard.clear();
- StateChangeNotification_Impl( false, nOldState, m_nObjectState );
+ StateChangeNotification_Impl( false, nOldState, m_nObjectState,
aGuard );
throw;
}
}
@@ -854,9 +850,7 @@ void SAL_CALL OleEmbeddedObject::doVerb( sal_Int32 nVerbID )
{
// if the target object is in loaded state
// it must be switched to running state to execute verb
- aGuard.clear();
- changeState( embed::EmbedStates::RUNNING );
- aGuard.reset();
+ ExecUnlocked([this]() { changeState(embed::EmbedStates::RUNNING);
}, aGuard);
}
try {
@@ -866,11 +860,13 @@ void SAL_CALL OleEmbeddedObject::doVerb( sal_Int32
nVerbID )
// ==== the STAMPIT related solution =============================
m_aVerbExecutionController.StartControlExecution();
- {
- ClearedMutexArea clearedMutex(aGuard);
- m_pOleComponent->ExecuteVerb(nVerbID);
- m_pOleComponent->SetHostName(m_aContainerName);
- }
+ ExecUnlocked(
+ [nVerbID, p = m_pOleComponent, name = m_aContainerName]()
+ {
+ p->ExecuteVerb(nVerbID);
+ p->SetHostName(name);
+ },
+ aGuard);
// ==== the STAMPIT related solution =============================
bool bModifiedOnExecution =
m_aVerbExecutionController.EndControlExecution_WasModified();
@@ -886,9 +882,7 @@ void SAL_CALL OleEmbeddedObject::doVerb( sal_Int32 nVerbID )
// ==== the STAMPIT related solution =============================
m_aVerbExecutionController.EndControlExecution_WasModified();
-
- aGuard.clear();
- StateChangeNotification_Impl( false, nOldState, m_nObjectState );
+ StateChangeNotification_Impl( false, nOldState, m_nObjectState,
aGuard );
throw;
}
@@ -1151,10 +1145,7 @@ sal_Int64 SAL_CALL OleEmbeddedObject::getStatus(
sal_Int64
nResult = m_nStatus;
else if ( m_pOleComponent )
{
- {
- ClearedMutexArea clearedMutex(aGuard);
- m_nStatus = m_pOleComponent->GetMiscStatus(nAspect);
- }
+ m_nStatus = ExecUnlocked([p = m_pOleComponent, nAspect] { return
p->GetMiscStatus(nAspect); }, aGuard);
m_nStatusAspect = nAspect;
m_bGotStatus = true;
nResult = m_nStatus;
diff --git a/embeddedobj/source/msole/olemisc.cxx
b/embeddedobj/source/msole/olemisc.cxx
index e2af0872f452..db661c0fb385 100644
--- a/embeddedobj/source/msole/olemisc.cxx
+++ b/embeddedobj/source/msole/olemisc.cxx
@@ -154,7 +154,8 @@ OleEmbeddedObject::~OleEmbeddedObject()
}
-void OleEmbeddedObject::MakeEventListenerNotification_Impl( const OUString&
aEventName )
+void OleEmbeddedObject::MakeEventListenerNotification_Impl( const OUString&
aEventName,
+
osl::ResettableMutexGuard& guard )
{
if ( !m_pInterfaceContainer )
return;
@@ -165,59 +166,59 @@ void
OleEmbeddedObject::MakeEventListenerNotification_Impl( const OUString& aEve
if ( pContainer == nullptr )
return;
- document::EventObject aEvent( static_cast< ::cppu::OWeakObject* >( this ),
aEventName );
- comphelper::OInterfaceIteratorHelper2 pIterator(*pContainer);
- while (pIterator.hasMoreElements())
+ auto proc = [&guard, aEvent = document::EventObject(getXWeak(),
aEventName)](
+ const uno::Reference<document::XEventListener>& xListener)
{
try
{
-
static_cast<document::XEventListener*>(pIterator.next())->notifyEvent( aEvent );
+ osl::ResettableMutexGuardScopedReleaser area(guard);
+ xListener->notifyEvent(aEvent);
}
- catch( const uno::RuntimeException& )
+ catch (const lang::DisposedException&)
{
+ throw; // forEach handles this
}
- }
+ catch (const uno::RuntimeException&)
+ {
+ }
+ };
+ pContainer->forEach<document::XEventListener>(proc);
}
#ifdef _WIN32
-void OleEmbeddedObject::StateChangeNotification_Impl( bool bBeforeChange,
sal_Int32 nOldState, sal_Int32 nNewState )
+void OleEmbeddedObject::StateChangeNotification_Impl( bool bBeforeChange,
sal_Int32 nOldState, sal_Int32 nNewState,
+
osl::ResettableMutexGuard& guard )
{
- if ( m_pInterfaceContainer )
+ if (!m_pInterfaceContainer)
+ return;
+
+ comphelper::OInterfaceContainerHelper2* pContainer =
m_pInterfaceContainer->getContainer(
+ cppu::UnoType<embed::XStateChangeListener>::get());
+ if (!pContainer)
+ return;
+
+ auto proc
+ = [bBeforeChange, nOldState, nNewState, &guard, aSource =
lang::EventObject(getXWeak())](
+ const uno::Reference<embed::XStateChangeListener>& xListener)
{
- comphelper::OInterfaceContainerHelper2* pContainer =
m_pInterfaceContainer->getContainer(
- cppu::UnoType<embed::XStateChangeListener>::get());
- if ( pContainer != nullptr )
+ try
{
- lang::EventObject aSource( static_cast< ::cppu::OWeakObject* >(
this ) );
- comphelper::OInterfaceIteratorHelper2 pIterator(*pContainer);
-
- while (pIterator.hasMoreElements())
- {
- if ( bBeforeChange )
- {
- try
- {
-
static_cast<embed::XStateChangeListener*>(pIterator.next())->changingState(
aSource, nOldState, nNewState );
- }
- catch( const uno::Exception& )
- {
- // even if the listener complains ignore it for now
- }
- }
- else
- {
- try
- {
-
static_cast<embed::XStateChangeListener*>(pIterator.next())->stateChanged(
aSource, nOldState, nNewState );
- }
- catch( const uno::Exception& )
- {
- // if anything happened it is problem of listener,
ignore it
- }
- }
- }
+ osl::ResettableMutexGuardScopedReleaser area(guard);
+ if (bBeforeChange)
+ xListener->changingState(aSource, nOldState, nNewState);
+ else
+ xListener->stateChanged(aSource, nOldState, nNewState);
}
- }
+ catch (const lang::DisposedException&)
+ {
+ throw; // forEach handles this
+ }
+ catch (const uno::Exception&)
+ {
+ // even if the listener complains ignore it for now
+ }
+ };
+ pContainer->forEach<embed::XStateChangeListener>(proc);
}
#endif
diff --git a/embeddedobj/source/msole/olepersist.cxx
b/embeddedobj/source/msole/olepersist.cxx
index f2f945d0ec24..fe53f5a35c36 100644
--- a/embeddedobj/source/msole/olepersist.cxx
+++ b/embeddedobj/source/msole/olepersist.cxx
@@ -822,7 +822,7 @@ bool OleEmbeddedObject::SaveObject_Impl()
bool OleEmbeddedObject::OnShowWindow_Impl( bool bShow )
{
- osl::ClearableMutexGuard aGuard(m_aMutex);
+ osl::ResettableMutexGuard aGuard(m_aMutex);
bool bResult = false;
@@ -838,21 +838,19 @@ bool OleEmbeddedObject::OnShowWindow_Impl( bool bShow )
m_nObjectState = embed::EmbedStates::ACTIVE;
m_aVerbExecutionController.ObjectIsActive();
- aGuard.clear();
- StateChangeNotification_Impl( false, nOldState, m_nObjectState );
+ StateChangeNotification_Impl( false, nOldState, m_nObjectState, aGuard
);
}
else if ( !bShow && m_nObjectState == embed::EmbedStates::ACTIVE )
{
m_nObjectState = embed::EmbedStates::RUNNING;
- aGuard.clear();
- StateChangeNotification_Impl( false, nOldState, m_nObjectState );
+ StateChangeNotification_Impl( false, nOldState, m_nObjectState, aGuard
);
}
if ( m_xClientSite.is() )
{
try
{
- m_xClientSite->visibilityChanged( bShow );
+ ExecUnlocked([p = m_xClientSite, bShow] {
p->visibilityChanged(bShow); }, aGuard);
bResult = true;
}
catch( const uno::Exception& )
@@ -873,6 +871,7 @@ void OleEmbeddedObject::OnIconChanged_Impl()
void OleEmbeddedObject::OnViewChanged_Impl()
{
+ osl::ResettableMutexGuard aGuard(m_aMutex);
if ( m_bDisposed )
throw lang::DisposedException();
@@ -896,7 +895,7 @@ void OleEmbeddedObject::OnViewChanged_Impl()
// The view is changed while the object is in running state, save the
new object
m_xCachedVisualRepresentation.clear();
SaveObject_Impl();
- MakeEventListenerNotification_Impl( "OnVisAreaChanged" );
+ MakeEventListenerNotification_Impl( "OnVisAreaChanged", aGuard );
}
}
@@ -904,6 +903,7 @@ void OleEmbeddedObject::OnViewChanged_Impl()
void OleEmbeddedObject::OnClosed_Impl()
{
+ osl::ResettableMutexGuard aGuard(m_aMutex);
if ( m_bDisposed )
throw lang::DisposedException();
@@ -911,7 +911,7 @@ void OleEmbeddedObject::OnClosed_Impl()
{
sal_Int32 nOldState = m_nObjectState;
m_nObjectState = embed::EmbedStates::LOADED;
- StateChangeNotification_Impl( false, nOldState, m_nObjectState );
+ StateChangeNotification_Impl( false, nOldState, m_nObjectState, aGuard
);
}
}
@@ -1114,7 +1114,7 @@ void OleEmbeddedObject::StoreToLocation_Impl(
#ifdef _WIN32
// if the object was NOT modified after storing it can be just copied
// as if it was in loaded state
- || (m_pOleComponent && !ExecUnlocked([this] { return
m_pOleComponent->IsDirty(); }, rGuard))
+ || (m_pOleComponent && !ExecUnlocked([p = m_pOleComponent] { return
p->IsDirty(); }, rGuard))
#endif
)
{
@@ -1537,7 +1537,7 @@ void SAL_CALL OleEmbeddedObject::saveCompleted( sal_Bool
bUseNew )
}
// end wrapping related part ====================
- osl::ClearableMutexGuard aGuard(m_aMutex);
+ osl::ResettableMutexGuard aGuard(m_aMutex);
if ( m_bDisposed )
throw lang::DisposedException(); // TODO
@@ -1606,16 +1606,15 @@ void SAL_CALL OleEmbeddedObject::saveCompleted(
sal_Bool bUseNew )
{}
}
- aGuard.clear();
if ( bUseNew )
{
- MakeEventListenerNotification_Impl( "OnSaveAsDone");
+ MakeEventListenerNotification_Impl( "OnSaveAsDone", aGuard);
// the object can be changed only on windows
// the notification should be done only if the object is not in loaded
state
if ( m_pOleComponent && m_nUpdateMode ==
embed::EmbedUpdateModes::ALWAYS_UPDATE && !bStoreLoaded )
{
- MakeEventListenerNotification_Impl( "OnVisAreaChanged");
+ MakeEventListenerNotification_Impl( "OnVisAreaChanged", aGuard);
}
}
}
@@ -1721,7 +1720,7 @@ void SAL_CALL OleEmbeddedObject::storeOwn()
bool bStoreLoaded = true;
#ifdef _WIN32
- if ( m_nObjectState != embed::EmbedStates::LOADED && m_pOleComponent &&
ExecUnlocked([this] { return m_pOleComponent->IsDirty(); }, aGuard) )
+ if ( m_nObjectState != embed::EmbedStates::LOADED && m_pOleComponent &&
ExecUnlocked([p = m_pOleComponent] { return p->IsDirty(); }, aGuard) )
{
bStoreLoaded = false;
@@ -1782,14 +1781,12 @@ void SAL_CALL OleEmbeddedObject::storeOwn()
{}
}
- aGuard.clear();
-
- MakeEventListenerNotification_Impl( "OnSaveDone");
+ MakeEventListenerNotification_Impl( "OnSaveDone", aGuard);
// the object can be changed only on Windows
// the notification should be done only if the object is not in loaded
state
if ( m_pOleComponent && m_nUpdateMode ==
embed::EmbedUpdateModes::ALWAYS_UPDATE && !bStoreLoaded )
- MakeEventListenerNotification_Impl( "OnVisAreaChanged");
+ MakeEventListenerNotification_Impl( "OnVisAreaChanged", aGuard);
}
commit 968e3be4d261f6d4f5698ee4acad5d1bac647855
Author: Mike Kaganski <mike.kagan...@collabora.com>
AuthorDate: Thu Aug 22 09:27:34 2024 +0500
Commit: Mike Kaganski <mike.kagan...@collabora.com>
CommitDate: Mon Sep 9 11:57:29 2024 +0500
Make OleEmbeddedObject::GetRidOfComponent safer when releasing lock
Thanks Michael Stahl for the useful advise - see
https://gerrit.libreoffice.org/c/core/+/171996/comment/4dc20148_5ec861ad/
Change-Id: I16e1876a81a52f291ede49572f012c15d4cd9a6d
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/172233
Tested-by: Jenkins
Reviewed-by: Mike Kaganski <mike.kagan...@collabora.com>
diff --git a/embeddedobj/source/msole/olemisc.cxx
b/embeddedobj/source/msole/olemisc.cxx
index ae4a292c37ce..e2af0872f452 100644
--- a/embeddedobj/source/msole/olemisc.cxx
+++ b/embeddedobj/source/msole/olemisc.cxx
@@ -230,23 +230,26 @@ void
OleEmbeddedObject::GetRidOfComponent(osl::ResettableMutexGuard* guard)
SaveObject_Impl();
m_pOleComponent->removeCloseListener( m_xClosePreventer );
+ // When releasing the guard below, avoid a case when two threads are
doing the same;
+ // store the reference on stack and clear m_pOleComponent in advance
+ rtl::Reference<OleComponent> pOleComponent(std::move(m_pOleComponent));
try
{
std::optional<osl::ResettableMutexGuardScopedReleaser> oReleaser;
if (guard)
oReleaser.emplace(*guard);
- m_pOleComponent->close( false );
+ pOleComponent->close(false);
}
catch( const uno::Exception& )
{
+ m_pOleComponent = std::move(pOleComponent);
// TODO: there should be a special listener to wait for component
closing
// and to notify object, may be object itself can be such a
listener
m_pOleComponent->addCloseListener( m_xClosePreventer );
throw;
}
- m_pOleComponent->disconnectEmbeddedObject();
- m_pOleComponent.clear();
+ pOleComponent->disconnectEmbeddedObject();
}
#else
(void)guard;
