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 <michael.meeks@collabora.com>
Tested-by: Michael Meeks <michael.meeks@collabora.com>

7 files changed, 41 insertions, 26 deletions

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<SotMutexHolder> 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<SotMutexHolder>& 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<SotMutexHolder>& 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<SotMutexHolder>& 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<SotMutexHolder>( 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<SotMutexHolder>& 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<SotMutexHolder>& 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<SotMutexHolder>& 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<SotMutexHolder>& 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<SotMutexHolder>& 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<io::XInputStream> 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<SotMutexHolder>& 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<SotMutexHolder>& 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<SotMutexHolder>& 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<SotMutexHolder>& 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<ZipFile>(m_xContentStream, m_xContext, true, m_bForceRecovery);
+                m_pZipFile = o3tl::make_unique<ZipFile>(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<ZipFile>(m_xContentStream, m_xContext, false);
+        m_pZipFile = o3tl::make_unique<ZipFile>(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<ZipFile>(
+                m_aMutexHolder,
                 m_xContentStream,
                 m_xContext,
                 true );