From d21a675d3a7deff37fd66adc993d0179d3a39ed7 Mon Sep 17 00:00:00 2001 From: Mohammed Abdul Azeem Date: Thu, 8 Jun 2017 13:32:42 +0530 Subject: Fixing loose ends for multithread Sync in package/: Mutexes in different classes operate exclusively, which might cause sync problem when multithreads are involved. This patch shares the mutex across all classes that share the underlying stream. Change-Id: I57e549fb7c375f93955bf54886b91b1892db1e27 Reviewed-on: https://gerrit.libreoffice.org/38563 Reviewed-by: Michael Meeks Tested-by: Michael Meeks --- package/inc/ZipFile.hxx | 9 ++++-- package/source/zipapi/XUnbufferedStream.cxx | 5 +-- package/source/zipapi/XUnbufferedStream.hxx | 1 + package/source/zipapi/ZipFile.cxx | 45 +++++++++++++++----------- package/source/zippackage/ZipPackage.cxx | 4 +-- package/source/zippackage/ZipPackageStream.cxx | 2 +- package/source/zippackage/zipfileaccess.cxx | 1 + 7 files changed, 41 insertions(+), 26 deletions(-) (limited to 'package') diff --git a/package/inc/ZipFile.hxx b/package/inc/ZipFile.hxx index e84ace2abd11..4aafaad77e6a 100644 --- a/package/inc/ZipFile.hxx +++ b/package/inc/ZipFile.hxx @@ -55,7 +55,7 @@ class ZipEnumeration; class ZipFile { - ::osl::Mutex m_aMutex; + rtl::Reference m_aMutexHolder; EntryHash aEntries; ByteGrabber aGrabber; @@ -90,11 +90,13 @@ class ZipFile public: - ZipFile( css::uno::Reference < css::io::XInputStream > &xInput, + ZipFile( const rtl::Reference& aMutexHolder, + css::uno::Reference < css::io::XInputStream > &xInput, const css::uno::Reference < css::uno::XComponentContext > &rxContext, bool bInitialise ); - ZipFile( css::uno::Reference < css::io::XInputStream > &xInput, + ZipFile( const rtl::Reference& aMutexHolder, + css::uno::Reference < css::io::XInputStream > &xInput, const css::uno::Reference < css::uno::XComponentContext > &rxContext, bool bInitialise, bool bForceRecover ); @@ -136,6 +138,7 @@ public: const css::uno::Reference < css::io::XInputStream >& rStream ); static css::uno::Reference< css::io::XInputStream > StaticGetDataFromRawStream( + const rtl::Reference& aMutexHolder, const css::uno::Reference< css::uno::XComponentContext >& rxContext, const css::uno::Reference< css::io::XInputStream >& xStream, const ::rtl::Reference < EncryptionData > &rData ); diff --git a/package/source/zipapi/XUnbufferedStream.cxx b/package/source/zipapi/XUnbufferedStream.cxx index 8992e227b24d..a7465e397e24 100644 --- a/package/source/zipapi/XUnbufferedStream.cxx +++ b/package/source/zipapi/XUnbufferedStream.cxx @@ -48,7 +48,7 @@ XUnbufferedStream::XUnbufferedStream( bool bIsEncrypted, const OUString& aMediaType, bool bRecoveryMode ) -: maMutexHolder( aMutexHolder.is() ? aMutexHolder : rtl::Reference( new SotMutexHolder ) ) +: maMutexHolder( aMutexHolder ) , mxZipStream ( xNewZipStream ) , mxZipSeek ( xNewZipStream, UNO_QUERY ) , maEntry ( rEntry ) @@ -108,9 +108,10 @@ XUnbufferedStream::XUnbufferedStream( // allows to read package raw stream XUnbufferedStream::XUnbufferedStream( const uno::Reference< uno::XComponentContext >& /*xContext*/, + const rtl::Reference& aMutexHolder, const Reference < XInputStream >& xRawStream, const ::rtl::Reference< EncryptionData >& rData ) -: maMutexHolder( new SotMutexHolder ) +: maMutexHolder( aMutexHolder ) , mxZipStream ( xRawStream ) , mxZipSeek ( xRawStream, UNO_QUERY ) , mnBlockSize( 1 ) diff --git a/package/source/zipapi/XUnbufferedStream.hxx b/package/source/zipapi/XUnbufferedStream.hxx index 02fe87f598de..d46320eac21d 100644 --- a/package/source/zipapi/XUnbufferedStream.hxx +++ b/package/source/zipapi/XUnbufferedStream.hxx @@ -77,6 +77,7 @@ public: // allows to read package raw stream XUnbufferedStream( const css::uno::Reference< css::uno::XComponentContext >& xContext, + const rtl::Reference& aMutexHolder, const css::uno::Reference < css::io::XInputStream >& xRawStream, const ::rtl::Reference< EncryptionData >& rData ); diff --git a/package/source/zipapi/ZipFile.cxx b/package/source/zipapi/ZipFile.cxx index 14b536db3ada..ba41d5f10d1d 100644 --- a/package/source/zipapi/ZipFile.cxx +++ b/package/source/zipapi/ZipFile.cxx @@ -70,8 +70,12 @@ using ZipUtils::Inflater; /** This class is used to read entries from a zip file */ -ZipFile::ZipFile( uno::Reference < XInputStream > &xInput, const uno::Reference < XComponentContext > & rxContext, bool bInitialise ) -: aGrabber(xInput) +ZipFile::ZipFile( const rtl::Reference& aMutexHolder, + uno::Reference < XInputStream > &xInput, + const uno::Reference < XComponentContext > & rxContext, + bool bInitialise ) +: m_aMutexHolder( aMutexHolder ) +, aGrabber( xInput ) , aInflater( true ) , xStream(xInput) , m_xContext ( rxContext ) @@ -88,8 +92,12 @@ ZipFile::ZipFile( uno::Reference < XInputStream > &xInput, const uno::Reference } } -ZipFile::ZipFile( uno::Reference < XInputStream > &xInput, const uno::Reference < XComponentContext > & rxContext, bool bInitialise, bool bForceRecovery) -: aGrabber(xInput) +ZipFile::ZipFile( const rtl::Reference& aMutexHolder, + uno::Reference < XInputStream > &xInput, + const uno::Reference < XComponentContext > & rxContext, + bool bInitialise, bool bForceRecovery) +: m_aMutexHolder( aMutexHolder ) +, aGrabber( xInput ) , aInflater( true ) , xStream(xInput) , m_xContext ( rxContext ) @@ -122,7 +130,7 @@ void ZipFile::setUseBufferedStream( bool b ) void ZipFile::setInputStream ( const uno::Reference < XInputStream >& xNewStream ) { - ::osl::MutexGuard aGuard( m_aMutex ); + ::osl::MutexGuard aGuard( m_aMutexHolder->GetMutex() ); xStream = xNewStream; aGrabber.setInputStream ( xStream ); @@ -381,7 +389,8 @@ bool ZipFile::StaticFillData ( ::rtl::Reference< BaseEncryptionData > & rData, return bOk; } -uno::Reference< XInputStream > ZipFile::StaticGetDataFromRawStream( const uno::Reference< uno::XComponentContext >& rxContext, +uno::Reference< XInputStream > ZipFile::StaticGetDataFromRawStream( const rtl::Reference& aMutexHolder, + const uno::Reference< uno::XComponentContext >& rxContext, const uno::Reference< XInputStream >& xStream, const ::rtl::Reference< EncryptionData > &rData ) { @@ -417,7 +426,7 @@ uno::Reference< XInputStream > ZipFile::StaticGetDataFromRawStream( const uno::R throw packages::WrongPasswordException(THROW_WHERE ); } - return new XUnbufferedStream( rxContext, xStream, rData ); + return new XUnbufferedStream( rxContext, aMutexHolder, xStream, rData ); } #if 0 @@ -490,7 +499,7 @@ bool ZipFile::StaticHasValidPassword( const uno::Reference< uno::XComponentConte bool ZipFile::hasValidPassword ( ZipEntry & rEntry, const ::rtl::Reference< EncryptionData >& rData ) { - ::osl::MutexGuard aGuard( m_aMutex ); + ::osl::MutexGuard aGuard( m_aMutexHolder->GetMutex() ); bool bRet = false; if ( rData.is() && rData->m_aKey.getLength() ) @@ -608,7 +617,7 @@ uno::Reference< XInputStream > ZipFile::createStreamForZipEntry( bool bIsEncrypted, const OUString& aMediaType ) { - ::osl::MutexGuard aGuard( m_aMutex ); + ::osl::MutexGuard aGuard( m_aMutexHolder->GetMutex() ); uno::Reference xSrcStream = new XUnbufferedStream( m_xContext, aMutexHolder, rEntry, xStream, rData, nStreamMode, bIsEncrypted, aMediaType, bRecoveryMode); @@ -630,7 +639,7 @@ uno::Reference< XInputStream > ZipFile::getInputStream( ZipEntry& rEntry, bool bIsEncrypted, const rtl::Reference& aMutexHolder ) { - ::osl::MutexGuard aGuard( m_aMutex ); + ::osl::MutexGuard aGuard( m_aMutexHolder->GetMutex() ); if ( rEntry.nOffset <= 0 ) readLOC( rEntry ); @@ -657,7 +666,7 @@ uno::Reference< XInputStream > ZipFile::getDataStream( ZipEntry& rEntry, bool bIsEncrypted, const rtl::Reference& aMutexHolder ) { - ::osl::MutexGuard aGuard( m_aMutex ); + ::osl::MutexGuard aGuard( m_aMutexHolder->GetMutex() ); if ( rEntry.nOffset <= 0 ) readLOC( rEntry ); @@ -693,7 +702,7 @@ uno::Reference< XInputStream > ZipFile::getRawData( ZipEntry& rEntry, bool bIsEncrypted, const rtl::Reference& aMutexHolder ) { - ::osl::MutexGuard aGuard( m_aMutex ); + ::osl::MutexGuard aGuard( m_aMutexHolder->GetMutex() ); if ( rEntry.nOffset <= 0 ) readLOC( rEntry ); @@ -707,7 +716,7 @@ uno::Reference< XInputStream > ZipFile::getWrappedRawStream( const OUString& aMediaType, const rtl::Reference& aMutexHolder ) { - ::osl::MutexGuard aGuard( m_aMutex ); + ::osl::MutexGuard aGuard( m_aMutexHolder->GetMutex() ); if ( !rData.is() ) throw packages::NoEncryptionException(THROW_WHERE ); @@ -720,7 +729,7 @@ uno::Reference< XInputStream > ZipFile::getWrappedRawStream( bool ZipFile::readLOC( ZipEntry &rEntry ) { - ::osl::MutexGuard aGuard( m_aMutex ); + ::osl::MutexGuard aGuard( m_aMutexHolder->GetMutex() ); sal_Int64 nPos = -rEntry.nOffset; @@ -948,7 +957,7 @@ sal_Int32 ZipFile::readCEN() void ZipFile::recover() { - ::osl::MutexGuard aGuard( m_aMutex ); + ::osl::MutexGuard aGuard( m_aMutexHolder->GetMutex() ); sal_Int64 nLength; Sequence < sal_Int8 > aBuffer; @@ -1123,7 +1132,7 @@ void ZipFile::recover() bool ZipFile::checkSizeAndCRC( const ZipEntry& aEntry ) { - ::osl::MutexGuard aGuard( m_aMutex ); + ::osl::MutexGuard aGuard( m_aMutexHolder->GetMutex() ); sal_Int32 nCRC = 0; sal_Int64 nSize = 0; @@ -1137,7 +1146,7 @@ bool ZipFile::checkSizeAndCRC( const ZipEntry& aEntry ) sal_Int32 ZipFile::getCRC( sal_Int64 nOffset, sal_Int64 nSize ) { - ::osl::MutexGuard aGuard( m_aMutex ); + ::osl::MutexGuard aGuard( m_aMutexHolder->GetMutex() ); Sequence < sal_Int8 > aBuffer; CRC32 aCRC; @@ -1157,7 +1166,7 @@ sal_Int32 ZipFile::getCRC( sal_Int64 nOffset, sal_Int64 nSize ) void ZipFile::getSizeAndCRC( sal_Int64 nOffset, sal_Int64 nCompressedSize, sal_Int64 *nSize, sal_Int32 *nCRC ) { - ::osl::MutexGuard aGuard( m_aMutex ); + ::osl::MutexGuard aGuard( m_aMutexHolder->GetMutex() ); Sequence < sal_Int8 > aBuffer; CRC32 aCRC; diff --git a/package/source/zippackage/ZipPackage.cxx b/package/source/zippackage/ZipPackage.cxx index 90272bbee62b..61faad5600c1 100644 --- a/package/source/zippackage/ZipPackage.cxx +++ b/package/source/zippackage/ZipPackage.cxx @@ -725,7 +725,7 @@ void SAL_CALL ZipPackage::initialize( const uno::Sequence< Any >& aArguments ) OUString message; try { - m_pZipFile = o3tl::make_unique(m_xContentStream, m_xContext, true, m_bForceRecovery); + m_pZipFile = o3tl::make_unique(m_aMutexHolder, m_xContentStream, m_xContext, true, m_bForceRecovery); m_pZipFile->setUseBufferedStream(bUseBufferedStream); getZipFileContents(); } @@ -1090,7 +1090,7 @@ void ZipPackage::ConnectTo( const uno::Reference< io::XInputStream >& xInStream if ( m_pZipFile ) m_pZipFile->setInputStream( m_xContentStream ); else - m_pZipFile = o3tl::make_unique(m_xContentStream, m_xContext, false); + m_pZipFile = o3tl::make_unique(m_aMutexHolder, m_xContentStream, m_xContext, false); } namespace diff --git a/package/source/zippackage/ZipPackageStream.cxx b/package/source/zippackage/ZipPackageStream.cxx index 01f2896d68f0..8607f4a2c583 100644 --- a/package/source/zippackage/ZipPackageStream.cxx +++ b/package/source/zippackage/ZipPackageStream.cxx @@ -1047,7 +1047,7 @@ uno::Reference< io::XInputStream > SAL_CALL ZipPackageStream::getDataStream() return xResult; } else if ( m_nStreamMode == PACKAGE_STREAM_RAW ) - return ZipFile::StaticGetDataFromRawStream( m_xContext, GetOwnSeekStream(), GetEncryptionData() ); + return ZipFile::StaticGetDataFromRawStream( m_rZipPackage.GetSharedMutexRef(), m_xContext, GetOwnSeekStream(), GetEncryptionData() ); else if ( GetOwnSeekStream().is() ) { return new WrapStreamForShare( GetOwnSeekStream(), m_rZipPackage.GetSharedMutexRef() ); diff --git a/package/source/zippackage/zipfileaccess.cxx b/package/source/zippackage/zipfileaccess.cxx index ce4e82de8867..7517e772caf2 100644 --- a/package/source/zippackage/zipfileaccess.cxx +++ b/package/source/zippackage/zipfileaccess.cxx @@ -247,6 +247,7 @@ void SAL_CALL OZipFileAccess::initialize( const uno::Sequence< uno::Any >& aArgu // TODO: in case xSeekable is implemented on separated XStream implementation a wrapper is required m_pZipFile = o3tl::make_unique( + m_aMutexHolder, m_xContentStream, m_xContext, true ); -- cgit