xmlhelp/source/cxxhelp/provider/databases.cxx | 120 ++++++++++++++++++--------
xmlhelp/source/cxxhelp/provider/databases.hxx | 27 ++++-
2 files changed, 104 insertions(+), 43 deletions(-)
New commits:
commit 5195530f3ac43f071bc5676fd7ad1a0adc63e9d8
Author: Michael Stahl <michael.st...@allotropia.de>
AuthorDate: Fri Apr 14 18:48:44 2023 +0200
Commit: Michael Stahl <michael.st...@allotropia.de>
CommitDate: Tue Apr 18 11:28:20 2023 +0200
xmlhelp: fix lots of deadlocks
Not sure if any of this makes sense but it doesn't deadlock immediatly.
(regression from comit 2c37a0ca31095165d05740a2ff4969f625b4a75b)
Change-Id: I4a5e01acb06b56a78453900a143d36cad1156f21
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/150431
Tested-by: Jenkins
Reviewed-by: Michael Stahl <michael.st...@allotropia.de>
diff --git a/xmlhelp/source/cxxhelp/provider/databases.cxx
b/xmlhelp/source/cxxhelp/provider/databases.cxx
index be5f964ca292..351625e7dbcb 100644
--- a/xmlhelp/source/cxxhelp/provider/databases.cxx
+++ b/xmlhelp/source/cxxhelp/provider/databases.cxx
@@ -277,6 +277,11 @@ OUString Databases::getInstallPathAsURL()
return m_aInstallDirectory;
}
+OUString Databases::getInstallPathAsURL(std::unique_lock<std::mutex>& )
+{
+ return m_aInstallDirectory;
+}
+
const std::vector< OUString >& Databases::getModuleList( const OUString&
Language )
{
if( m_avModules.empty() )
@@ -430,20 +435,29 @@ OUString Databases::processLang(
std::unique_lock<std::mutex>& /*rGuard*/, const
return ret;
}
-helpdatafileproxy::Hdf* Databases::getHelpDataFile( std::u16string_view
Database,
+helpdatafileproxy::Hdf* Databases::getHelpDataFile(std::u16string_view
Database,
const OUString& Language, bool helpText,
const OUString* pExtensionPath )
+{
+ std::unique_lock aGuard( m_aMutex );
+
+ return getHelpDataFile(aGuard, Database, Language, helpText,
pExtensionPath);
+}
+
+helpdatafileproxy::Hdf*
Databases::getHelpDataFile(std::unique_lock<std::mutex>& rGuard,
+ std::u16string_view Database,
+ const OUString& Language, bool helpText,
+ const OUString* pExtensionPath )
+
{
if( Database.empty() || Language.isEmpty() )
return nullptr;
- std::unique_lock aGuard( m_aMutex );
-
OUString aFileExt( helpText ? OUString(".ht") : OUString(".db") );
OUString dbFileName = OUString::Concat("/") + Database + aFileExt;
OUString key;
if( pExtensionPath == nullptr )
- key = processLang( aGuard, Language ) + dbFileName;
+ key = processLang( rGuard, Language ) + dbFileName;
else
key = *pExtensionPath + Language + dbFileName; // make unique,
don't change language
@@ -458,7 +472,7 @@ helpdatafileproxy::Hdf* Databases::getHelpDataFile(
std::u16string_view Database
OUString fileURL;
if( pExtensionPath )
- fileURL = expandURL(aGuard, *pExtensionPath) + Language +
dbFileName;
+ fileURL = expandURL(rGuard, *pExtensionPath) + Language +
dbFileName;
else
fileURL = m_aInstallDirectory + key;
@@ -480,12 +494,10 @@ helpdatafileproxy::Hdf* Databases::getHelpDataFile(
std::u16string_view Database
}
Reference< XCollator >
-Databases::getCollator( const OUString& Language )
+Databases::getCollator(std::unique_lock<std::mutex>&, const OUString& Language)
{
OUString key = Language;
- std::unique_lock aGuard( m_aMutex );
-
CollatorTable::iterator it =
m_aCollatorTable.emplace( key, Reference< XCollator >() ).first;
@@ -728,7 +740,7 @@ KeywordInfo* Databases::getKeyword( const OUString&
Database,
bool bExtension = false;
for (;;)
{
- fileURL = aDbFileIt.nextDbFile( bExtension );
+ fileURL = aDbFileIt.nextDbFile(aGuard, bExtension);
if( fileURL.isEmpty() )
break;
OUString fileNameHDFHelp( fileURL );
@@ -741,7 +753,7 @@ KeywordInfo* Databases::getKeyword( const OUString&
Database,
helpdatafileproxy::HDFData aValue;
if( aHdf.startIteration() )
{
- helpdatafileproxy::Hdf* pHdf = getHelpDataFile(
Database,Language );
+ helpdatafileproxy::Hdf* pHdf = getHelpDataFile(aGuard,
Database,Language );
if( pHdf != nullptr )
{
pHdf->releaseHashMap();
@@ -776,7 +788,7 @@ KeywordInfo* Databases::getKeyword( const OUString&
Database,
}
// sorting
- Reference< XCollator > xCollator = getCollator( Language );
+ Reference<XCollator> xCollator = getCollator(aGuard, Language);
KeywordElementComparator aComparator( xCollator );
std::sort(aVector.begin(),aVector.end(),aComparator);
@@ -786,7 +798,8 @@ KeywordInfo* Databases::getKeyword( const OUString&
Database,
return it->second.get();
}
-Reference< XHierarchicalNameAccess > Databases::jarFile( std::u16string_view
jar,
+Reference< XHierarchicalNameAccess > Databases::jarFile(
+ std::unique_lock<std::mutex>& rGuard, std::u16string_view jar,
const OUString&
Language )
{
if( jar.empty() || Language.isEmpty() )
@@ -794,9 +807,7 @@ Reference< XHierarchicalNameAccess > Databases::jarFile(
std::u16string_view jar
return Reference< XHierarchicalNameAccess >( nullptr );
}
- std::unique_lock aGuard( m_aMutex );
-
- OUString key = processLang(aGuard, Language) + "/" + jar;
+ OUString key = processLang(rGuard, Language) + "/" + jar;
ZipFileTable::iterator it =
m_aZipFileTable.emplace( key,Reference< XHierarchicalNameAccess
>(nullptr) ).first;
@@ -814,7 +825,7 @@ Reference< XHierarchicalNameAccess > Databases::jarFile(
std::u16string_view jar
std::u16string_view aExtensionPath = jar.substr(
nQuestionMark1 + 1, nQuestionMark2 - nQuestionMark1 - 1 );
std::u16string_view aPureJar = jar.substr( nQuestionMark2 + 1
);
- zipFile = expandURL( aGuard, OUString::Concat(aExtensionPath)
+ "/" + aPureJar );
+ zipFile = expandURL(rGuard, OUString::Concat(aExtensionPath) +
"/" + aPureJar);
}
else
{
@@ -878,12 +889,14 @@ Reference< XHierarchicalNameAccess >
Databases::findJarFileForPath
return xNA;
}
+ ::std::unique_lock aGuard(m_aMutex);
+
JarFileIterator aJarFileIt( m_xContext, *this, jar, Language );
Reference< XHierarchicalNameAccess > xTestNA;
Reference< deployment::XPackage > xParentPackageBundle;
for (;;)
{
- xTestNA = aJarFileIt.nextJarFile( xParentPackageBundle,
o_pExtensionPath, o_pExtensionRegistryPath );
+ xTestNA = aJarFileIt.nextJarFile(aGuard, xParentPackageBundle,
o_pExtensionPath, o_pExtensionRegistryPath);
if( !xTestNA.is() )
break;
if( xTestNA.is() && xTestNA->hasByHierarchicalName( path ) )
@@ -1291,6 +1304,40 @@ Reference< deployment::XPackage >
ExtensionIteratorBase::implGetNextBundledHelpP
return xHelpPackage;
}
+OUString ExtensionIteratorBase::implGetFileFromPackage(
+ std::unique_lock<std::mutex> & rGuard,
+ std::u16string_view rFileExtension, const Reference< deployment::XPackage
>& xPackage )
+{
+ // No extension -> search for pure language folder
+ bool bLangFolderOnly = rFileExtension.empty();
+
+ OUString aFile;
+ OUString aLanguage = m_aLanguage;
+ for( sal_Int32 iPass = 0 ; iPass < 2 ; ++iPass )
+ {
+ OUString aStr = xPackage->getRegistrationDataURL().Value + "/" +
aLanguage;
+ if( !bLangFolderOnly )
+ {
+ aStr += OUString::Concat("/help") + rFileExtension;
+ }
+
+ aFile = m_rDatabases.expandURL(rGuard, aStr);
+ if( iPass == 0 )
+ {
+ if( m_xSFA->exists( aFile ) )
+ break;
+
+ ::std::vector< OUString > av;
+ implGetLanguageVectorFromPackage( av, xPackage );
+ ::std::vector< OUString >::const_iterator pFound =
LanguageTag::getFallback( av, m_aLanguage );
+ if( pFound != av.end() )
+ aLanguage = *pFound;
+ }
+ }
+ return aFile;
+}
+
+
OUString ExtensionIteratorBase::implGetFileFromPackage(
std::u16string_view rFileExtension, const Reference< deployment::XPackage
>& xPackage )
{
@@ -1467,7 +1514,7 @@ helpdatafileproxy::Hdf*
DataBaseIterator::implGetHdfFromPackage( const Reference
//returns a file URL
-OUString KeyDataBaseFileIterator::nextDbFile( bool& o_rbExtension )
+OUString KeyDataBaseFileIterator::nextDbFile(std::unique_lock<std::mutex>&
rGuard, bool& o_rbExtension)
{
OUString aRetFile;
@@ -1476,8 +1523,8 @@ OUString KeyDataBaseFileIterator::nextDbFile( bool&
o_rbExtension )
switch( m_eState )
{
case IteratorState::InitialModule:
- aRetFile = m_rDatabases.getInstallPathAsURL() +
- m_rDatabases.processLang(m_aLanguage) +
+ aRetFile = m_rDatabases.getInstallPathAsURL(rGuard) +
+ m_rDatabases.processLang(rGuard, m_aLanguage) +
"/" +
m_aInitialModule + ".key";
@@ -1497,7 +1544,7 @@ OUString KeyDataBaseFileIterator::nextDbFile( bool&
o_rbExtension )
if( !xHelpPackage.is() )
break;
- aRetFile = implGetDbFileFromPackage( xHelpPackage );
+ aRetFile = implGetDbFileFromPackage(rGuard, xHelpPackage);
o_rbExtension = true;
break;
}
@@ -1509,7 +1556,7 @@ OUString KeyDataBaseFileIterator::nextDbFile( bool&
o_rbExtension )
if( !xHelpPackage.is() )
break;
- aRetFile = implGetDbFileFromPackage( xHelpPackage );
+ aRetFile = implGetDbFileFromPackage(rGuard, xHelpPackage);
o_rbExtension = true;
break;
}
@@ -1521,7 +1568,7 @@ OUString KeyDataBaseFileIterator::nextDbFile( bool&
o_rbExtension )
if( !xHelpPackage.is() )
break;
- aRetFile = implGetDbFileFromPackage( xHelpPackage );
+ aRetFile = implGetDbFileFromPackage(rGuard, xHelpPackage);
o_rbExtension = true;
break;
}
@@ -1536,18 +1583,20 @@ OUString KeyDataBaseFileIterator::nextDbFile( bool&
o_rbExtension )
}
//Returns a file URL, that does not contain macros
-OUString KeyDataBaseFileIterator::implGetDbFileFromPackage
- ( const Reference< deployment::XPackage >& xPackage )
+OUString KeyDataBaseFileIterator::implGetDbFileFromPackage(
+ std::unique_lock<std::mutex>& rGuard,
+ const Reference<deployment::XPackage>& xPackage)
{
OUString aExpandedURL =
- implGetFileFromPackage( u".key", xPackage );
+ implGetFileFromPackage(rGuard, u".key", xPackage);
return aExpandedURL;
}
-Reference< XHierarchicalNameAccess > JarFileIterator::nextJarFile
- ( Reference< deployment::XPackage >& o_xParentPackageBundle,
+Reference<XHierarchicalNameAccess> JarFileIterator::nextJarFile(
+ std::unique_lock<std::mutex>& rGuard,
+ Reference< deployment::XPackage >& o_xParentPackageBundle,
OUString* o_pExtensionPath, OUString* o_pExtensionRegistryPath )
{
Reference< XHierarchicalNameAccess > xNA;
@@ -1557,7 +1606,7 @@ Reference< XHierarchicalNameAccess >
JarFileIterator::nextJarFile
switch( m_eState )
{
case IteratorState::InitialModule:
- xNA = m_rDatabases.jarFile( m_aInitialModule, m_aLanguage );
+ xNA = m_rDatabases.jarFile(rGuard, m_aInitialModule,
m_aLanguage);
m_eState = IteratorState::UserExtensions; // Later:
SHARED_MODULE
break;
@@ -1571,7 +1620,7 @@ Reference< XHierarchicalNameAccess >
JarFileIterator::nextJarFile
if( !xHelpPackage.is() )
break;
- xNA = implGetJarFromPackage( xHelpPackage, o_pExtensionPath,
o_pExtensionRegistryPath );
+ xNA = implGetJarFromPackage(rGuard, xHelpPackage,
o_pExtensionPath, o_pExtensionRegistryPath);
break;
}
@@ -1581,7 +1630,7 @@ Reference< XHierarchicalNameAccess >
JarFileIterator::nextJarFile
if( !xHelpPackage.is() )
break;
- xNA = implGetJarFromPackage( xHelpPackage, o_pExtensionPath,
o_pExtensionRegistryPath );
+ xNA = implGetJarFromPackage(rGuard, xHelpPackage,
o_pExtensionPath, o_pExtensionRegistryPath);
break;
}
@@ -1591,7 +1640,7 @@ Reference< XHierarchicalNameAccess >
JarFileIterator::nextJarFile
if( !xHelpPackage.is() )
break;
- xNA = implGetJarFromPackage( xHelpPackage, o_pExtensionPath,
o_pExtensionRegistryPath );
+ xNA = implGetJarFromPackage(rGuard, xHelpPackage,
o_pExtensionPath, o_pExtensionRegistryPath);
break;
}
@@ -1604,13 +1653,14 @@ Reference< XHierarchicalNameAccess >
JarFileIterator::nextJarFile
return xNA;
}
-Reference< XHierarchicalNameAccess > JarFileIterator::implGetJarFromPackage
-( const Reference< deployment::XPackage >& xPackage, OUString*
o_pExtensionPath, OUString* o_pExtensionRegistryPath )
+Reference< XHierarchicalNameAccess > JarFileIterator::implGetJarFromPackage(
+ std::unique_lock<std::mutex>& rGuard,
+ const Reference<deployment::XPackage>& xPackage, OUString*
o_pExtensionPath, OUString* o_pExtensionRegistryPath)
{
Reference< XHierarchicalNameAccess > xNA;
OUString zipFile =
- implGetFileFromPackage( u".jar", xPackage );
+ implGetFileFromPackage(rGuard, u".jar", xPackage);
try
{
diff --git a/xmlhelp/source/cxxhelp/provider/databases.hxx
b/xmlhelp/source/cxxhelp/provider/databases.hxx
index 1711cba5278c..448d1f92cd71 100644
--- a/xmlhelp/source/cxxhelp/provider/databases.hxx
+++ b/xmlhelp/source/cxxhelp/provider/databases.hxx
@@ -146,6 +146,7 @@ namespace chelp {
OString getImageTheme() const;
OUString getInstallPathAsURL();
+ OUString getInstallPathAsURL(std::unique_lock<std::mutex>& rGuard);
const std::vector< OUString >& getModuleList( const OUString& Language
);
@@ -159,12 +160,17 @@ namespace chelp {
helpdatafileproxy::Hdf* getHelpDataFile( std::u16string_view Module,
const OUString& Language, bool helpText = false,
const OUString* pExtensionPath = nullptr );
+ helpdatafileproxy::Hdf* getHelpDataFile(std::unique_lock<std::mutex>&
rGuard,
+ std::u16string_view Module,
+ const OUString& Language, bool helpText = false,
+ const OUString* pExtensionPath = nullptr );
+
/**
* The following method returns the Collator for the given
language-country combination
*/
css::uno::Reference< css::i18n::XCollator >
- getCollator( const OUString& Language );
+ getCollator(std::unique_lock<std::mutex>& rGuard, const OUString&
Language);
/**
* Returns the cascading style sheet used to format the HTML-output.
@@ -194,7 +200,7 @@ namespace chelp {
*/
css::uno::Reference< css::container::XHierarchicalNameAccess >
- jarFile( std::u16string_view jar,
+ jarFile(std::unique_lock<std::mutex>& rGuard, std::u16string_view jar,
const OUString& Language );
css::uno::Reference< css::container::XHierarchicalNameAccess >
@@ -206,6 +212,7 @@ namespace chelp {
* Maps a given language-locale combination to language or locale.
*/
OUString processLang( const OUString& Language );
+ OUString processLang( std::unique_lock<std::mutex>& rGuard, const
OUString& Language );
void replaceName( OUString& oustring ) const;
@@ -213,13 +220,12 @@ namespace chelp {
const OUString& getProductVersion() const { return m_vReplacement[1]; }
OUString expandURL( const OUString& aURL );
+ OUString expandURL( std::unique_lock<std::mutex>& rGuard, const
OUString& aURL );
static OUString expandURL( const OUString& aURL,
const css::uno::Reference< css::uno::XComponentContext >& xContext
);
private:
- OUString expandURL( std::unique_lock<std::mutex>& rGuard, const
OUString& aURL );
- OUString processLang( std::unique_lock<std::mutex>& rGuard, const
OUString& Language );
std::mutex m_aMutex;
css::uno::Reference< css::uno::XComponentContext > m_xContext;
@@ -327,6 +333,9 @@ namespace chelp {
( css::uno::Reference< css::deployment::XPackage >&
o_xParentPackageBundle );
OUString implGetFileFromPackage( std::u16string_view rFileExtension,
const css::uno::Reference< css::deployment::XPackage >& xPackage );
+ OUString implGetFileFromPackage(std::unique_lock<std::mutex>& rGuard,
+ std::u16string_view rFileExtension,
+ const css::uno::Reference< css::deployment::XPackage >& xPackage );
void implGetLanguageVectorFromPackage( ::std::vector< OUString > &rv,
const css::uno::Reference< css::deployment::XPackage >& xPackage );
@@ -390,10 +399,10 @@ namespace chelp {
: ExtensionIteratorBase( xContext, rDatabases, aInitialModule,
aLanguage )
{}
//Returns a file URL
- OUString nextDbFile( bool& o_rbExtension );
+ OUString nextDbFile(std::unique_lock<std::mutex>& rGuard, bool&
o_rbExtension);
private:
- OUString implGetDbFileFromPackage(
+ OUString implGetDbFileFromPackage(std::unique_lock<std::mutex>& rGuard,
const css::uno::Reference< css::deployment::XPackage >& xPackage );
}; // end class KeyDataBaseFileIterator
@@ -407,12 +416,14 @@ namespace chelp {
{}
css::uno::Reference< css::container::XHierarchicalNameAccess >
- nextJarFile( css::uno::Reference< css::deployment::XPackage >&
o_xParentPackageBundle,
+ nextJarFile(std::unique_lock<std::mutex>& rGuard,
+ css::uno::Reference<css::deployment::XPackage>&
o_xParentPackageBundle,
OUString* o_pExtensionPath, OUString*
o_pExtensionRegistryPath );
private:
css::uno::Reference< css::container::XHierarchicalNameAccess >
- implGetJarFromPackage(const css::uno::Reference<
css::deployment::XPackage >& xPackage,
+ implGetJarFromPackage(std::unique_lock<std::mutex>& rGuard,
+ const css::uno::Reference< css::deployment::XPackage >&
xPackage,
OUString* o_pExtensionPath, OUString* o_pExtensionRegistryPath
);
}; // end class JarFileIterator
