embeddedobj/source/inc/oleembobj.hxx | 6 +++++- embeddedobj/source/msole/olepersist.cxx | 13 +++++++------ embeddedobj/source/msole/olevisual.cxx | 17 +++++++++++++---- xmlsecurity/qa/unit/signing/signing.cxx | 10 +++++----- 4 files changed, 30 insertions(+), 16 deletions(-)
New commits: commit 83460af06025a23aec327cc46b2d808da6011039 Author: Mike Kaganski <[email protected]> AuthorDate: Thu Aug 15 15:55:44 2024 +0500 Commit: Mike Kaganski <[email protected]> CommitDate: Tue Oct 15 12:30:55 2024 +0500 Fix the test failing when invalid certificate is in cert store testPDFAddVisibleSignature was failing for me locally because of an expired certificate present in my store. Change-Id: I03243f6707b1b5ca94ea87e9f8c809dd47b6a93a Reviewed-on: https://gerrit.libreoffice.org/c/core/+/171901 Tested-by: Jenkins Reviewed-by: Mike Kaganski <[email protected]> (cherry picked from commit 2ca75d4133e2388b67d09a9d88969cd20dc68f26) Reviewed-on: https://gerrit.libreoffice.org/c/core/+/172078 Reviewed-by: Xisco Fauli <[email protected]> diff --git a/xmlsecurity/qa/unit/signing/signing.cxx b/xmlsecurity/qa/unit/signing/signing.cxx index 316eaf22f5b7..c0b44cc403e4 100644 --- a/xmlsecurity/qa/unit/signing/signing.cxx +++ b/xmlsecurity/qa/unit/signing/signing.cxx @@ -748,21 +748,21 @@ CPPUNIT_TEST_FIXTURE(SigningTest, testPDFAddVisibleSignature) uno::Reference<view::XSelectionSupplier> xSelectionSupplier(pBaseModel->getCurrentController(), uno::UNO_QUERY); xSelectionSupplier->select(uno::Any(xShape)); - uno::Sequence<uno::Reference<security::XCertificate>> aCertificates - = mxSecurityContext->getSecurityEnvironment()->getPersonalCertificates(); - if (!aCertificates.hasElements()) + auto xEnv = mxSecurityContext->getSecurityEnvironment(); + auto xCert = GetValidCertificate(xEnv->getPersonalCertificates(), xEnv); + if (!xCert) { return; } SdrView* pView = SfxViewShell::Current()->GetDrawView(); - svx::SignatureLineHelper::setShapeCertificate(pView, aCertificates[0]); + svx::SignatureLineHelper::setShapeCertificate(pView, xCert); // the document is modified now, but Sign function can't show SaveAs dialog // in unit test, so just clear the modified pObjectShell->SetModified(false); // When: do the actual signing. - pObjectShell->SignDocumentContentUsingCertificate(aCertificates[0]); + pObjectShell->SignDocumentContentUsingCertificate(xCert); // Then: count the # of shapes on the signature widget/annotation. std::unique_ptr<vcl::pdf::PDFiumDocument> pPdfDocument = parsePDFExport(); commit bbc52df1f0e33c9c8a210cc34657d443883bf3ce Author: Mike Kaganski <[email protected]> AuthorDate: Mon Oct 14 15:14:26 2024 +0500 Commit: Mike Kaganski <[email protected]> CommitDate: Tue Oct 15 09:52:06 2024 +0500 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-repombeddedobj\source\msole\oleembed.cxx @ 643] sfxlo!SfxInPlaceClient::IsObjectUIActive+0x23 [libre-office-git-repo\sfx2\source iew\ipclient.cxx @ 847] sfxlo!SfxViewShell::GetUIActiveClient+0x5b [libre-office-git-repo\sfx2\source iew iewsh.cxx @ 2521] sfxlo!SfxDispatcher::FindServer_+0x282 [libre-office-git-repo\sfx2\source sfxlo!SfxDispatcher::GetShellAndSlot_Impl+0x2f [libre-office-git-repo\sfx2\source sfxlo!SfxDispatcher::QueryState+0x5b [libre-office-git-repo\sfx2\source swlo!SwView::CheckReadonlyState+0x64 [libre-office-git-repo\sw\source\uibase\uiview iew.cxx @ 627] swlo!SwView::TimeoutHdl+0x76 [libre-office-git-repo\sw\source\uibase\uiview iew.cxx @ 608] vcllo!Scheduler::CallbackTaskScheduling+0x130e [libre-office-git-repo cl\sourcepp\scheduler.cxx @ 511] vclplug_winlo!WinSalTimer::ImplHandleElapsedTimer+0x2e [libre-office-git-repo cl\winpp\saltimer.cxx @ 167] vclplug_winlo!ImplSalYield+0x14f [libre-office-git-repo cl\winpp\salinst.cxx @ 525] vclplug_winlo!WinSalInstance::DoYield+0xad [libre-office-git-repo cl\winpp\salinst.cxx @ 581] vcllo!ImplYield+0x367 [libre-office-git-repo cl\sourcepp\svapp.cxx @ 393] vcllo!Application::Execute+0xfa [libre-office-git-repo cl\sourcepp\svapp.cxx @ 369] sofficeapp!desktop::Desktop::Main+0x173f [libre-office-git-repo\desktop\sourcepppp.cxx @ 1605] vcllo!ImplSVMain+0xda [libre-office-git-repo cl\sourcepp\svmain.cxx @ 229] sofficeapp!soffice_main+0xf3 [libre-office-git-repo\desktop\sourcepp\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 cl\winpp\salinst.cxx @ 148] emboleobj!SolarMutexReleaser::{dtor}+0xc [libre-office-git-repo\include cl\svapp.hxx @ 1425] emboleobj!OleComponent::GetExtent+0x102 [libre-office-git-repombeddedobj\source\msole\olecomponent.cxx @ 1134] emboleobj!OleEmbeddedObject::getVisualAreaSize+0x23f [libre-office-git-repombeddedobj\source\msole\olevisual.cxx @ 227] emboleobj!OleEmbeddedObject::saveCompleted+0x406 [libre-office-git-repombeddedobj\source\msole\olepersist.cxx @ 1603] comphelper!comphelper::EmbeddedObjectContainer::StoreEmbeddedObject+0x49b [libre-office-git-repo comphelper!comphelper::EmbeddedObjectContainer::InsertEmbeddedObject+0x6f [libre-office-git-repo comphelper!comphelper::EmbeddedObjectContainer::RemoveEmbeddedObject+0x388 [libre-office-git-repo comphelper!comphelper::EmbeddedObjectContainer::RemoveEmbeddedObject+0x46 [libre-office-git-repo swlo!SwOLENode::SavePersistentData+0x20b [libre-office-git-repo\sw\source swlo!SwNodes::ChgNode+0x411 [libre-office-git-repo\sw\source swlo!SwNodes::MoveNodes+0x18e6 [libre-office-git-repo\sw\source swlo!SwUndoSaveContent::MoveToUndoNds+0x1c1 [libre-office-git-repo\sw\source swlo!SwUndoSaveSection::SaveSection+0x559 [libre-office-git-repo\sw\source swlo!SwUndoSaveSection::SaveSection+0x55 [libre-office-git-repo\sw\source swlo!SwUndoFlyBase::DelFly+0x127 [libre-office-git-repo\sw\source swlo!SwUndoDelLayFormat::SwUndoDelLayFormat+0x64 [libre-o ffice-git-repo\sw\source swlo!std::make_unique+0x22 [C:\Program Files (x86)\Microsoft Visual Studio�9\BuildTools\VC\Tools\MSVC .29.30133\include\memory @ 3382] swlo!sw::DocumentLayoutManager::DelLayoutFormat+0x283 [libre-office-git-repo\sw\source swlo!SwTextNode::DestroyAttr+0x82 [libre-office-git-repo\sw\source swlo!SwTextNode::EraseText+0x18f [libre-office-git-repo\sw\source swlo!SwTextNode::DeleteAttributes+0x115 [libre-office-git-repo\sw\source swlo!SwXFrame::dispose+0xdb [libre-office-git-repo\sw\source mscx_uno!`anonymous namespace'::cpp_call+0x710 [libre-office-git-reporidges\source mscx_uno!unoInterfaceProxyDispatch+0x2fa [libre-office-git-reporidges\source binaryurplo!binaryurp::IncomingRequest::execute_throw+0x635 [libre-office-git-repoinaryurp\source\incomingrequest.cxx @ 239] binaryurplo!binaryurp::IncomingRequest::execute+0xbf [libre-office-git-repoinaryurp\source\incomingrequest.cxx @ 79] binaryurplo!request+0x1c [libre-office-git-repoinaryurp\source eader.cxx @ 84] cppu3!cppu_threadpool::JobQueue::enter+0x21e [libre-office-git-repo cppu3!cppu_threadpool::ORequestThread::run+0xa0 [libre-office-git-repo 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 <[email protected]> diff --git a/embeddedobj/source/inc/oleembobj.hxx b/embeddedobj/source/inc/oleembobj.hxx index 0e18a168f00e..d1eda24fe302 100644 --- a/embeddedobj/source/inc/oleembobj.hxx +++ b/embeddedobj/source/inc/oleembobj.hxx @@ -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); }; class ClearedMutexArea diff --git a/embeddedobj/source/msole/olepersist.cxx b/embeddedobj/source/msole/olepersist.cxx index fe53f5a35c36..25649707118c 100644 --- a/embeddedobj/source/msole/olepersist.cxx +++ b/embeddedobj/source/msole/olepersist.cxx @@ -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& ) {} diff --git a/embeddedobj/source/msole/olevisual.cxx b/embeddedobj/source/msole/olevisual.cxx index 075f02998257..2266e85de270 100644 --- a/embeddedobj/source/msole/olevisual.cxx +++ b/embeddedobj/source/msole/olevisual.cxx @@ -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 ====================
