compilerplugins/clang/test/unnecessarygetstr.cxx | 7
compilerplugins/clang/unnecessarygetstr.cxx | 18 +
filter/source/config/cache/filtercache.cxx | 224 ++++++++++++-----------
filter/source/config/cache/filtercache.hxx | 41 +---
4 files changed, 161 insertions(+), 129 deletions(-)
New commits:
commit 3d5c0a94539d2196c7d0dd9f52660ba9e58d31b8
Author: Noel Grandin <noel.gran...@collabora.co.uk>
AuthorDate: Fri Feb 2 10:49:14 2024 +0200
Commit: Noel Grandin <noel.gran...@collabora.co.uk>
CommitDate: Fri Feb 2 20:40:58 2024 +0100
loplugin:unnecessarygetstr fix false +
spotted in https://gerrit.libreoffice.org/c/core/+/162869
Change-Id: I87d9fdcfed5282f0e94fc8aa95a46054883fdd79
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/162929
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.gran...@collabora.co.uk>
diff --git a/compilerplugins/clang/test/unnecessarygetstr.cxx
b/compilerplugins/clang/test/unnecessarygetstr.cxx
index c0960557a89b..bdb58cde2ef7 100644
--- a/compilerplugins/clang/test/unnecessarygetstr.cxx
+++ b/compilerplugins/clang/test/unnecessarygetstr.cxx
@@ -10,6 +10,7 @@
#include <sal/config.h>
#include <ostream>
+#include <sstream>
#include <string_view>
#include <string>
@@ -128,4 +129,10 @@ void foo(const OString&);
void test(std::string v) { foo(v.c_str()); }
}
+// no warning expected
+namespace test7
+{
+void test(const OString& v) { std::stringstream aStream(v.getStr()); }
+}
+
/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s
cinkeys+=0=break: */
diff --git a/compilerplugins/clang/unnecessarygetstr.cxx
b/compilerplugins/clang/unnecessarygetstr.cxx
index c80877a78554..589ab405f786 100644
--- a/compilerplugins/clang/unnecessarygetstr.cxx
+++ b/compilerplugins/clang/unnecessarygetstr.cxx
@@ -11,6 +11,7 @@
#include <cassert>
#include <stack>
+#include <unordered_set>
#include "check.hxx"
#include "plugin.hxx"
@@ -60,11 +61,20 @@ public:
if (ignoreLocation(constructExpr))
return true;
auto tc = loplugin::TypeCheck(constructExpr->getType());
- if (tc.ClassOrStruct("basic_string").StdNamespace())
+ if (tc.ClassOrStruct("basic_stringstream").StdNamespace())
+ {
+ // ignore the implicit-conversion nodes that are added here
+ if (constructExpr->getNumArgs() > 0)
+
nodesToIgnore.insert(constructExpr->getArg(0)->IgnoreImplicit());
+ }
+ else if (tc.ClassOrStruct("basic_string").StdNamespace())
{
if (constructExpr->getNumArgs() == 1 ||
constructExpr->getNumArgs() == 2)
- checkForGetStr(constructExpr->getArg(0), "string constructor",
- /*isOStringConstructor*/ false);
+ {
+ if (nodesToIgnore.find(constructExpr) == nodesToIgnore.end())
+ checkForGetStr(constructExpr->getArg(0), "string
constructor",
+ /*isOStringConstructor*/ false);
+ }
}
else if (tc.ClassOrStruct("basic_string_view").StdNamespace())
{
@@ -138,6 +148,8 @@ private:
TraverseDecl(compiler.getASTContext().getTranslationUnitDecl());
}
}
+
+ std::unordered_set<const Expr*> nodesToIgnore;
};
loplugin::Plugin::Registration<UnnecessaryGetStr>
unnecessarygetstr("unnecessarygetstr");
commit b23aec2266da4cc2b3f4a34037d3a073a36d3d83
Author: Noel Grandin <noel.gran...@collabora.co.uk>
AuthorDate: Fri Feb 2 18:51:59 2024 +0200
Commit: Noel Grandin <noel.gran...@collabora.co.uk>
CommitDate: Fri Feb 2 20:40:45 2024 +0100
tdf#159461 deadlock in Dialog "XML Filter Settings"
revert
commit 15405dc68b2e88b53585578567da13e3e99962db
Author: Noel Grandin <noel.gran...@collabora.co.uk>
Date: Mon Feb 20 15:43:41 2023 +0200
osl::Mutex->std::mutex in FilterCache
There is much recursive behaviour going on here,
via the configmgr and the listeners that FilterCache
has.
Change-Id: Ice003404dffad9e5e63bcff30c2ceede4f52cab8
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/162930
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.gran...@collabora.co.uk>
diff --git a/filter/source/config/cache/filtercache.cxx
b/filter/source/config/cache/filtercache.cxx
index 70c18a841bb4..2abf11a5ada9 100644
--- a/filter/source/config/cache/filtercache.cxx
+++ b/filter/source/config/cache/filtercache.cxx
@@ -119,7 +119,7 @@ FilterCache::~FilterCache()
std::unique_ptr<FilterCache> FilterCache::clone() const
{
// SAFE -> ----------------------------------
- std::unique_lock aGuard(m_aMutex);
+ osl::MutexGuard aLock(m_aMutex);
auto pClone = std::make_unique<FilterCache>();
@@ -151,7 +151,7 @@ std::unique_ptr<FilterCache> FilterCache::clone() const
void FilterCache::takeOver(const FilterCache& rClone)
{
// SAFE -> ----------------------------------
- std::unique_lock aGuard(m_aMutex);
+ osl::MutexGuard aLock(m_aMutex);
// a)
// Don't copy the configuration access points here!
@@ -192,14 +192,14 @@ void FilterCache::takeOver(const FilterCache& rClone)
// Because we can't be sure, that changed filters on one clone
// and changed types of another clone work together.
// But here we can check against the later changes...
- impl_validateAndOptimize(aGuard);
+ impl_validateAndOptimize();
// <- SAFE ----------------------------------
}
void FilterCache::load(EFillState eRequired)
{
// SAFE -> ----------------------------------
- std::unique_lock aGuard(m_aMutex);
+ osl::MutexGuard aLock(m_aMutex);
// check if required fill state is already reached ...
// There is nothing to do then.
@@ -217,28 +217,28 @@ void FilterCache::load(EFillState eRequired)
// office document with a minimal set of values.
if (m_eFillState == E_CONTAINS_NOTHING)
{
- impl_getDirectCFGValue(aGuard, CFGDIRECTKEY_OFFICELOCALE) >>=
m_sActLocale;
+ impl_getDirectCFGValue(CFGDIRECTKEY_OFFICELOCALE) >>= m_sActLocale;
if (m_sActLocale.isEmpty())
{
m_sActLocale = DEFAULT_OFFICELOCALE;
}
// Support the old configuration support. Read it only one times
during office runtime!
- impl_readOldFormat(aGuard);
+ impl_readOldFormat();
}
// b) If the required fill state was not reached
// but std values was already loaded ...
// we must load some further missing items.
- impl_load(aGuard, eRequired);
+ impl_load(eRequired);
// <- SAFE
}
bool FilterCache::isFillState(FilterCache::EFillState eState) const
{
// SAFE ->
- std::unique_lock aGuard(m_aMutex);
+ osl::MutexGuard aLock(m_aMutex);
return ((m_eFillState & eState) == eState);
// <- SAFE
}
@@ -249,12 +249,12 @@ std::vector<OUString>
FilterCache::getMatchingItemsByProps( EItemType eTyp
std::span< const
css::beans::NamedValue > lEProps) const
{
// SAFE ->
- std::unique_lock aGuard(m_aMutex);
+ osl::MutexGuard aLock(m_aMutex);
// search for right list
// An exception is thrown - "eType" is unknown.
// => rList will be valid everytimes next line is reached.
- const CacheItemList& rList = impl_getItemList(aGuard, eType);
+ const CacheItemList& rList = impl_getItemList(eType);
std::vector<OUString> lKeys;
lKeys.reserve(rList.size());
@@ -280,12 +280,12 @@ std::vector<OUString>
FilterCache::getMatchingItemsByProps( EItemType eTyp
bool FilterCache::hasItems(EItemType eType) const
{
// SAFE ->
- std::unique_lock aGuard(m_aMutex);
+ osl::MutexGuard aLock(m_aMutex);
// search for right list
// An exception is thrown - "eType" is unknown.
// => rList will be valid everytimes next line is reached.
- const CacheItemList& rList = impl_getItemList(aGuard, eType);
+ const CacheItemList& rList = impl_getItemList(eType);
return !rList.empty();
// <- SAFE
@@ -295,17 +295,12 @@ bool FilterCache::hasItems(EItemType eType) const
std::vector<OUString> FilterCache::getItemNames(EItemType eType) const
{
// SAFE ->
- std::unique_lock aGuard(m_aMutex);
- return getItemNames(aGuard, eType);
- // <- SAFE
-}
+ osl::MutexGuard aLock(m_aMutex);
-std::vector<OUString> FilterCache::getItemNames(std::unique_lock<std::mutex>&
rGuard, EItemType eType) const
-{
// search for right list
// An exception is thrown - "eType" is unknown.
// => rList will be valid everytimes next line is reached.
- const CacheItemList& rList = impl_getItemList(rGuard, eType);
+ const CacheItemList& rList = impl_getItemList(eType);
std::vector<OUString> lKeys;
for (auto const& elem : rList)
@@ -313,6 +308,7 @@ std::vector<OUString>
FilterCache::getItemNames(std::unique_lock<std::mutex>& rG
lKeys.push_back(elem.first);
}
return lKeys;
+ // <- SAFE
}
@@ -320,12 +316,12 @@ bool FilterCache::hasItem( EItemType eType,
const OUString& sItem)
{
// SAFE ->
- std::unique_lock aGuard(m_aMutex);
+ osl::MutexGuard aLock(m_aMutex);
// search for right list
// An exception is thrown - "eType" is unknown.
// => rList will be valid everytimes next line is reached.
- const CacheItemList& rList = impl_getItemList(aGuard, eType);
+ const CacheItemList& rList = impl_getItemList(eType);
// if item could not be found - check if it can be loaded
// from the underlying configuration layer. Might it was not already
@@ -336,7 +332,7 @@ bool FilterCache::hasItem( EItemType eType,
try
{
- impl_loadItemOnDemand(aGuard, eType, sItem);
+ impl_loadItemOnDemand(eType, sItem);
// no exception => item could be loaded!
return true;
}
@@ -352,22 +348,21 @@ CacheItem FilterCache::getItem( EItemType
eType,
const OUString& sItem)
{
// SAFE ->
- std::unique_lock aGuard(m_aMutex);
+ osl::MutexGuard aLock(m_aMutex);
- CacheItem aItem = impl_getItem(aGuard, eType, sItem);
+ CacheItem aItem = impl_getItem(eType, sItem);
// <- SAFE
return aItem;
}
-CacheItem& FilterCache::impl_getItem( std::unique_lock<std::mutex>& rGuard,
- EItemType eType,
+CacheItem& FilterCache::impl_getItem( EItemType eType,
const OUString& sItem)
{
// search for right list
// An exception is thrown if "eType" is unknown.
// => rList will be valid everytimes next line is reached.
- CacheItemList& rList = impl_getItemList(rGuard, eType);
+ CacheItemList& rList = impl_getItemList(eType);
// check if item exists ...
CacheItemList::iterator pIt = rList.find(sItem);
@@ -377,7 +372,7 @@ CacheItem& FilterCache::impl_getItem(
std::unique_lock<std::mutex>& rGuard,
// underlying configuration layer.
// Note: NoSuchElementException is thrown automatically here if
// item could not be loaded!
- pIt = impl_loadItemOnDemand(rGuard, eType, sItem);
+ pIt = impl_loadItemOnDemand(eType, sItem);
}
/* Workaround for #137955#
@@ -395,7 +390,7 @@ CacheItem& FilterCache::impl_getItem(
std::unique_lock<std::mutex>& rGuard,
// but it is there to load help pages
bool bIsHelpFilter = sItem == "writer_web_HTML_help";
- if ( !bIsHelpFilter && !impl_isModuleInstalled(rGuard, sDocService) )
+ if ( !bIsHelpFilter && !impl_isModuleInstalled(sDocService) )
{
OUString sMsg("The requested filter '" + sItem +
"' exists ... but it should not; because the
corresponding LibreOffice module was not installed.");
@@ -411,16 +406,16 @@ void FilterCache::removeItem( EItemType eType,
const OUString& sItem)
{
// SAFE ->
- std::unique_lock aGuard(m_aMutex);
+ osl::MutexGuard aLock(m_aMutex);
// search for right list
// An exception is thrown - "eType" is unknown.
// => rList will be valid everytimes next line is reached.
- CacheItemList& rList = impl_getItemList(aGuard, eType);
+ CacheItemList& rList = impl_getItemList(eType);
CacheItemList::iterator pItem = rList.find(sItem);
if (pItem == rList.end())
- pItem = impl_loadItemOnDemand(aGuard, eType, sItem); // throws
NoSuchELementException!
+ pItem = impl_loadItemOnDemand(eType, sItem); // throws
NoSuchELementException!
rList.erase(pItem);
impl_addItem2FlushList(eType, sItem);
@@ -432,12 +427,12 @@ void FilterCache::setItem( EItemType eType ,
const CacheItem& aValue)
{
// SAFE ->
- std::unique_lock aGuard(m_aMutex);
+ osl::MutexGuard aLock(m_aMutex);
// search for right list
// An exception is thrown - "eType" is unknown.
// => rList will be valid everytimes next line is reached.
- CacheItemList& rList = impl_getItemList(aGuard, eType);
+ CacheItemList& rList = impl_getItemList(eType);
// name must be part of the property set too ... otherwise our
// container query can't work correctly
@@ -459,8 +454,8 @@ void FilterCache::refreshItem( EItemType eType,
const OUString& sItem)
{
// SAFE ->
- std::unique_lock aGuard(m_aMutex);
- impl_loadItemOnDemand(aGuard, eType, sItem);
+ osl::MutexGuard aLock(m_aMutex);
+ impl_loadItemOnDemand(eType, sItem);
}
@@ -468,9 +463,9 @@ css::uno::Any FilterCache::getItemWithStateProps(
EItemType eType,
const OUString& sItem)
{
// SAFE ->
- std::unique_lock aGuard(m_aMutex);
+ osl::MutexGuard aLock(m_aMutex);
- const CacheItem& rItem = impl_getItem(aGuard, eType, sItem);
+ const CacheItem& rItem = impl_getItem(eType, sItem);
// Note: Opening of the configuration layer throws some exceptions
// if it failed. So we mustn't check any reference here...
@@ -480,14 +475,14 @@ css::uno::Any FilterCache::getItemWithStateProps(
EItemType eType,
{
case E_TYPE :
{
- xPackage.set(impl_openConfig(aGuard, E_PROVIDER_TYPES),
css::uno::UNO_QUERY_THROW);
+ xPackage.set(impl_openConfig(E_PROVIDER_TYPES),
css::uno::UNO_QUERY_THROW);
xPackage->getByName(CFGSET_TYPES) >>= xSet;
}
break;
case E_FILTER :
{
- xPackage.set(impl_openConfig(aGuard, E_PROVIDER_FILTERS),
css::uno::UNO_QUERY_THROW);
+ xPackage.set(impl_openConfig(E_PROVIDER_FILTERS),
css::uno::UNO_QUERY_THROW);
xPackage->getByName(CFGSET_FILTERS) >>= xSet;
}
break;
@@ -501,7 +496,7 @@ css::uno::Any FilterCache::getItemWithStateProps(
EItemType eType,
about FINALIZED and MANDATORY very easy ... :-(
=> set it to readonly/required everytimes :-)
*/
- css::uno::Any aDirectValue =
impl_getDirectCFGValue(aGuard, CFGDIRECTKEY_DEFAULTFRAMELOADER);
+ css::uno::Any aDirectValue =
impl_getDirectCFGValue(CFGDIRECTKEY_DEFAULTFRAMELOADER);
OUString sDefaultFrameLoader;
if (
(aDirectValue >>= sDefaultFrameLoader) &&
@@ -514,14 +509,14 @@ css::uno::Any FilterCache::getItemWithStateProps(
EItemType eType,
}
/* <-- HACK */
- xPackage.set(impl_openConfig(aGuard, E_PROVIDER_OTHERS),
css::uno::UNO_QUERY_THROW);
+ xPackage.set(impl_openConfig(E_PROVIDER_OTHERS),
css::uno::UNO_QUERY_THROW);
xPackage->getByName(CFGSET_FRAMELOADERS) >>= xSet;
}
break;
case E_CONTENTHANDLER :
{
- xPackage.set(impl_openConfig(aGuard, E_PROVIDER_OTHERS),
css::uno::UNO_QUERY_THROW);
+ xPackage.set(impl_openConfig(E_PROVIDER_OTHERS),
css::uno::UNO_QUERY_THROW);
xPackage->getByName(CFGSET_CONTENTHANDLERS) >>= xSet;
}
break;
@@ -575,14 +570,14 @@ void FilterCache::removeStatePropsFromItem(CacheItem&
rItem)
void FilterCache::flush()
{
// SAFE ->
- std::unique_lock aGuard(m_aMutex);
+ osl::MutexGuard aLock(m_aMutex);
// renew all dependencies and optimizations
- impl_validateAndOptimize(aGuard);
+ impl_validateAndOptimize();
if (!m_lChangedTypes.empty())
{
- css::uno::Reference< css::container::XNameAccess >
xConfig(impl_openConfig(aGuard, E_PROVIDER_TYPES), css::uno::UNO_QUERY_THROW);
+ css::uno::Reference< css::container::XNameAccess >
xConfig(impl_openConfig(E_PROVIDER_TYPES), css::uno::UNO_QUERY_THROW);
css::uno::Reference< css::container::XNameAccess > xSet ;
xConfig->getByName(CFGSET_TYPES) >>= xSet;
@@ -594,7 +589,7 @@ void FilterCache::flush()
if (!m_lChangedFilters.empty())
{
- css::uno::Reference< css::container::XNameAccess >
xConfig(impl_openConfig(aGuard, E_PROVIDER_FILTERS), css::uno::UNO_QUERY_THROW);
+ css::uno::Reference< css::container::XNameAccess >
xConfig(impl_openConfig(E_PROVIDER_FILTERS), css::uno::UNO_QUERY_THROW);
css::uno::Reference< css::container::XNameAccess > xSet ;
xConfig->getByName(CFGSET_FILTERS) >>= xSet;
@@ -677,7 +672,7 @@ void FilterCache::detectFlatForURL(const css::util::URL&
aURL ,
sExtension = sExtension.toAsciiLowerCase();
// SAFE -> ----------------------------------
- std::unique_lock aGuard(m_aMutex);
+ osl::MutexGuard aLock(m_aMutex);
// i) Step over all well known URL pattern
@@ -723,8 +718,11 @@ void FilterCache::detectFlatForURL(const css::util::URL&
aURL ,
// <- SAFE ----------------------------------
}
-const CacheItemList&
FilterCache::impl_getItemList(std::unique_lock<std::mutex>& /*rGuard*/,
EItemType eType) const
+const CacheItemList& FilterCache::impl_getItemList(EItemType eType) const
{
+ // SAFE -> ----------------------------------
+ osl::MutexGuard aLock(m_aMutex);
+
switch(eType)
{
case E_TYPE : return m_lTypes ;
@@ -736,10 +734,14 @@ const CacheItemList&
FilterCache::impl_getItemList(std::unique_lock<std::mutex>&
throw css::uno::RuntimeException("unknown sub container requested.",
css::uno::Reference<
css::uno::XInterface >());
+ // <- SAFE ----------------------------------
}
-CacheItemList& FilterCache::impl_getItemList(std::unique_lock<std::mutex>&
/*rGuard*/, EItemType eType)
+CacheItemList& FilterCache::impl_getItemList(EItemType eType)
{
+ // SAFE -> ----------------------------------
+ osl::MutexGuard aLock(m_aMutex);
+
switch(eType)
{
case E_TYPE : return m_lTypes ;
@@ -751,10 +753,13 @@ CacheItemList&
FilterCache::impl_getItemList(std::unique_lock<std::mutex>& /*rGu
throw css::uno::RuntimeException("unknown sub container requested.",
css::uno::Reference<
css::uno::XInterface >());
+ // <- SAFE ----------------------------------
}
-css::uno::Reference< css::uno::XInterface >
FilterCache::impl_openConfig(std::unique_lock<std::mutex>& rGuard,
EConfigProvider eProvider)
+css::uno::Reference< css::uno::XInterface >
FilterCache::impl_openConfig(EConfigProvider eProvider)
{
+ osl::MutexGuard aLock(m_aMutex);
+
OUString sPath ;
css::uno::Reference< css::uno::XInterface >* pConfig = nullptr;
css::uno::Reference< css::uno::XInterface > xOld ;
@@ -807,7 +812,7 @@ css::uno::Reference< css::uno::XInterface >
FilterCache::impl_openConfig(std::un
{
SAL_INFO( "filter.config", "" << sRtlLog);
- *pConfig = impl_createConfigAccess(rGuard, sPath ,
+ *pConfig = impl_createConfigAccess(sPath ,
false, // bReadOnly
true ); // bLocalesMode
}
@@ -835,7 +840,7 @@ css::uno::Reference< css::uno::XInterface >
FilterCache::impl_openConfig(std::un
return *pConfig;
}
-css::uno::Any
FilterCache::impl_getDirectCFGValue(std::unique_lock<std::mutex>& rGuard,
std::u16string_view sDirectKey)
+css::uno::Any FilterCache::impl_getDirectCFGValue(std::u16string_view
sDirectKey)
{
OUString sRoot;
OUString sKey ;
@@ -847,7 +852,7 @@ css::uno::Any
FilterCache::impl_getDirectCFGValue(std::unique_lock<std::mutex>&
)
return css::uno::Any();
- css::uno::Reference< css::uno::XInterface > xCfg =
impl_createConfigAccess(rGuard, sRoot ,
+ css::uno::Reference< css::uno::XInterface > xCfg =
impl_createConfigAccess(sRoot ,
true , // bReadOnly
false); // bLocalesMode
if (!xCfg.is())
@@ -874,11 +879,13 @@ css::uno::Any
FilterCache::impl_getDirectCFGValue(std::unique_lock<std::mutex>&
}
-css::uno::Reference< css::uno::XInterface >
FilterCache::impl_createConfigAccess(std::unique_lock<std::mutex>& /*rGuard*/,
-
const OUString& sRoot ,
+css::uno::Reference< css::uno::XInterface >
FilterCache::impl_createConfigAccess(const OUString& sRoot ,
bool bReadOnly ,
bool bLocalesMode)
{
+ // SAFE ->
+ osl::MutexGuard aLock(m_aMutex);
+
css::uno::Reference< css::uno::XInterface > xCfg;
if (!comphelper::IsFuzzing())
@@ -930,11 +937,15 @@ css::uno::Reference< css::uno::XInterface >
FilterCache::impl_createConfigAccess
}
return xCfg;
+ // <- SAFE
}
-void FilterCache::impl_validateAndOptimize(std::unique_lock<std::mutex>&
rGuard)
+void FilterCache::impl_validateAndOptimize()
{
+ // SAFE ->
+ osl::MutexGuard aLock(m_aMutex);
+
// First check if any filter or type could be read
// from the underlying configuration!
bool bSomeTypesShouldExist = ((m_eFillState & E_CONTAINS_STANDARD
) == E_CONTAINS_STANDARD );
@@ -1148,7 +1159,7 @@ void
FilterCache::impl_validateAndOptimize(std::unique_lock<std::mutex>& rGuard)
// create dependencies between the global default frame loader
// and all types (and of course if registered filters), which
// does not registered for any other loader.
- css::uno::Any aDirectValue = impl_getDirectCFGValue(rGuard,
CFGDIRECTKEY_DEFAULTFRAMELOADER);
+ css::uno::Any aDirectValue =
impl_getDirectCFGValue(CFGDIRECTKEY_DEFAULTFRAMELOADER);
OUString sDefaultFrameLoader;
if (
@@ -1164,7 +1175,7 @@ void
FilterCache::impl_validateAndOptimize(std::unique_lock<std::mutex>& rGuard)
// b) step over all well known frame loader services
// and remove all types from list a), which already
// referenced by a loader b)
- std::vector<OUString> lTypes = getItemNames(rGuard, E_TYPE);
+ std::vector<OUString> lTypes = getItemNames(E_TYPE);
for (auto & frameLoader : m_lFrameLoaders)
{
// Note: of course the default loader must be ignored here.
@@ -1201,6 +1212,8 @@ void
FilterCache::impl_validateAndOptimize(std::unique_lock<std::mutex>& rGuard)
#if OSL_DEBUG_LEVEL > 0
OSL_ENSURE(!nWarnings,
OUStringToOString(sLogOut,RTL_TEXTENCODING_UTF8).getStr());
#endif
+
+ // <- SAFE
}
void FilterCache::impl_addItem2FlushList( EItemType eType,
@@ -1255,8 +1268,11 @@ FilterCache::EItemFlushState
FilterCache::impl_specifyFlushOperation(const css::
return eState;
}
-void FilterCache::impl_load(std::unique_lock<std::mutex>& rGuard, EFillState
eRequiredState)
+void FilterCache::impl_load(EFillState eRequiredState)
{
+ // SAFE ->
+ osl::MutexGuard aLock(m_aMutex);
+
// Attention: Detect services are part of the standard set!
// So there is no need to handle it separately.
@@ -1270,10 +1286,10 @@ void
FilterCache::impl_load(std::unique_lock<std::mutex>& rGuard, EFillState eRe
// Attention! If config couldn't be opened successfully
// and exception is thrown automatically and must be forwarded
// to our caller...
- css::uno::Reference< css::container::XNameAccess >
xTypes(impl_openConfig(rGuard, E_PROVIDER_TYPES), css::uno::UNO_QUERY_THROW);
+ css::uno::Reference< css::container::XNameAccess >
xTypes(impl_openConfig(E_PROVIDER_TYPES), css::uno::UNO_QUERY_THROW);
{
SAL_INFO( "filter.config", "FilterCache::load std");
- impl_loadSet(rGuard, xTypes, E_TYPE, E_READ_STANDARD, &m_lTypes);
+ impl_loadSet(xTypes, E_TYPE, E_READ_STANDARD, &m_lTypes);
}
}
@@ -1287,10 +1303,10 @@ void
FilterCache::impl_load(std::unique_lock<std::mutex>& rGuard, EFillState eRe
// Attention! If config couldn't be opened successfully
// and exception is thrown automatically and must be forwarded
// to our call...
- css::uno::Reference< css::container::XNameAccess >
xTypes(impl_openConfig(rGuard, E_PROVIDER_TYPES), css::uno::UNO_QUERY_THROW);
+ css::uno::Reference< css::container::XNameAccess >
xTypes(impl_openConfig(E_PROVIDER_TYPES), css::uno::UNO_QUERY_THROW);
{
SAL_INFO( "filter.config", "FilterCache::load all types");
- impl_loadSet(rGuard, xTypes, E_TYPE, E_READ_UPDATE, &m_lTypes);
+ impl_loadSet(xTypes, E_TYPE, E_READ_UPDATE, &m_lTypes);
}
}
@@ -1304,10 +1320,10 @@ void
FilterCache::impl_load(std::unique_lock<std::mutex>& rGuard, EFillState eRe
// Attention! If config couldn't be opened successfully
// and exception is thrown automatically and must be forwarded
// to our call...
- css::uno::Reference< css::container::XNameAccess >
xFilters(impl_openConfig(rGuard, E_PROVIDER_FILTERS),
css::uno::UNO_QUERY_THROW);
+ css::uno::Reference< css::container::XNameAccess >
xFilters(impl_openConfig(E_PROVIDER_FILTERS), css::uno::UNO_QUERY_THROW);
{
SAL_INFO( "filter.config", "FilterCache::load all filters");
- impl_loadSet(rGuard, xFilters, E_FILTER, E_READ_ALL, &m_lFilters);
+ impl_loadSet(xFilters, E_FILTER, E_READ_ALL, &m_lFilters);
}
}
@@ -1321,10 +1337,10 @@ void
FilterCache::impl_load(std::unique_lock<std::mutex>& rGuard, EFillState eRe
// Attention! If config couldn't be opened successfully
// and exception is thrown automatically and must be forwarded
// to our call...
- css::uno::Reference< css::container::XNameAccess >
xLoaders(impl_openConfig(rGuard, E_PROVIDER_OTHERS), css::uno::UNO_QUERY_THROW);
+ css::uno::Reference< css::container::XNameAccess >
xLoaders(impl_openConfig(E_PROVIDER_OTHERS), css::uno::UNO_QUERY_THROW);
{
SAL_INFO( "filter.config", "FilterCache::load all frame loader");
- impl_loadSet(rGuard, xLoaders, E_FRAMELOADER, E_READ_ALL,
&m_lFrameLoaders);
+ impl_loadSet(xLoaders, E_FRAMELOADER, E_READ_ALL,
&m_lFrameLoaders);
}
}
@@ -1338,10 +1354,10 @@ void
FilterCache::impl_load(std::unique_lock<std::mutex>& rGuard, EFillState eRe
// Attention! If config couldn't be opened successfully
// and exception is thrown automatically and must be forwarded
// to our call...
- css::uno::Reference< css::container::XNameAccess >
xHandlers(impl_openConfig(rGuard, E_PROVIDER_OTHERS),
css::uno::UNO_QUERY_THROW);
+ css::uno::Reference< css::container::XNameAccess >
xHandlers(impl_openConfig(E_PROVIDER_OTHERS), css::uno::UNO_QUERY_THROW);
{
SAL_INFO( "filter.config", "FilterCache::load all content
handler");
- impl_loadSet(rGuard, xHandlers, E_CONTENTHANDLER, E_READ_ALL,
&m_lContentHandlers);
+ impl_loadSet(xHandlers, E_CONTENTHANDLER, E_READ_ALL,
&m_lContentHandlers);
}
}
@@ -1350,11 +1366,12 @@ void
FilterCache::impl_load(std::unique_lock<std::mutex>& rGuard, EFillState eRe
// any data read?
// yes! => validate it and update optimized structures.
- impl_validateAndOptimize(rGuard);
+ impl_validateAndOptimize();
+
+ // <- SAFE
}
-void FilterCache::impl_loadSet(std::unique_lock<std::mutex>& rGuard,
- const css::uno::Reference<
css::container::XNameAccess >& xConfig,
+void FilterCache::impl_loadSet(const css::uno::Reference<
css::container::XNameAccess >& xConfig,
EItemType
eType ,
EReadOption
eOption,
CacheItemList*
pCache )
@@ -1421,7 +1438,7 @@ void
FilterCache::impl_loadSet(std::unique_lock<std::mutex>& rGuard,
{
try
{
- (*pCache)[pItems[i]] = impl_loadItem(rGuard, xSet, eType,
pItems[i], eOption);
+ (*pCache)[pItems[i]] = impl_loadItem(xSet, eType,
pItems[i], eOption);
}
catch(const css::uno::Exception& ex)
{
@@ -1445,7 +1462,7 @@ void
FilterCache::impl_loadSet(std::unique_lock<std::mutex>& rGuard,
}
try
{
- CacheItem aItem = impl_loadItem(rGuard, xSet, eType,
pItems[i], eOption);
+ CacheItem aItem = impl_loadItem(xSet, eType, pItems[i],
eOption);
pItem->second.update(aItem);
}
catch(const css::uno::Exception& ex)
@@ -1462,12 +1479,15 @@ void
FilterCache::impl_loadSet(std::unique_lock<std::mutex>& rGuard,
}
}
-void FilterCache::impl_readPatchUINames(std::unique_lock<std::mutex>&
/*rGuard*/,
- const css::uno::Reference<
css::container::XNameAccess >& xNode,
- CacheItem&
rItem)
+void FilterCache::impl_readPatchUINames(const css::uno::Reference<
css::container::XNameAccess >& xNode,
+ CacheItem&
rItem)
{
+ // SAFE -> ----------------------------------
+ osl::ClearableMutexGuard aLock(m_aMutex);
OUString sActLocale = m_sActLocale ;
+ aLock.clear();
+ // <- SAFE ----------------------------------
css::uno::Any aVal = xNode->getByName(PROPNAME_UINAME);
css::uno::Reference< css::container::XNameAccess > xUIName;
@@ -1537,8 +1557,7 @@ void FilterCache::impl_savePatchUINames(const
css::uno::Reference< css::containe
will force a crash during destruction)
can be solved ...
-----------------------------------------------*/
-CacheItem FilterCache::impl_loadItem(std::unique_lock<std::mutex>& rGuard,
- const css::uno::Reference<
css::container::XNameAccess >& xSet ,
+CacheItem FilterCache::impl_loadItem(const css::uno::Reference<
css::container::XNameAccess >& xSet ,
EItemType
eType ,
const OUString&
sItem ,
EReadOption
eOption)
@@ -1582,7 +1601,7 @@ CacheItem
FilterCache::impl_loadItem(std::unique_lock<std::mutex>& rGuard,
// read optional properties of a type
// no else here! Is an additional switch ...
if (eOption == E_READ_UPDATE || eOption == E_READ_ALL)
- impl_readPatchUINames(rGuard, xItem, aItem);
+ impl_readPatchUINames(xItem, aItem);
}
break;
@@ -1619,7 +1638,7 @@ CacheItem
FilterCache::impl_loadItem(std::unique_lock<std::mutex>& rGuard,
// will be finished really
#ifdef AS_ENABLE_FILTER_UINAMES
if (eOption == E_READ_UPDATE || eOption == E_READ_ALL)
- impl_readPatchUINames(rGuard, xItem, aItem);
+ impl_readPatchUINames(xItem, aItem);
#endif // AS_ENABLE_FILTER_UINAMES
}
break;
@@ -1634,8 +1653,7 @@ CacheItem
FilterCache::impl_loadItem(std::unique_lock<std::mutex>& rGuard,
return aItem;
}
-CacheItemList::iterator FilterCache::impl_loadItemOnDemand(
std::unique_lock<std::mutex>& rGuard,
- EItemType
eType,
+CacheItemList::iterator FilterCache::impl_loadItemOnDemand( EItemType
eType,
const OUString&
sItem)
{
CacheItemList* pList = nullptr;
@@ -1647,7 +1665,7 @@ CacheItemList::iterator
FilterCache::impl_loadItemOnDemand( std::unique_lock<std
case E_TYPE :
{
pList = &m_lTypes;
- xConfig = impl_openConfig(rGuard, E_PROVIDER_TYPES);
+ xConfig = impl_openConfig(E_PROVIDER_TYPES);
sSet = CFGSET_TYPES;
}
break;
@@ -1655,7 +1673,7 @@ CacheItemList::iterator
FilterCache::impl_loadItemOnDemand( std::unique_lock<std
case E_FILTER :
{
pList = &m_lFilters;
- xConfig = impl_openConfig(rGuard, E_PROVIDER_FILTERS);
+ xConfig = impl_openConfig(E_PROVIDER_FILTERS);
sSet = CFGSET_FILTERS;
}
break;
@@ -1663,7 +1681,7 @@ CacheItemList::iterator
FilterCache::impl_loadItemOnDemand( std::unique_lock<std
case E_FRAMELOADER :
{
pList = &m_lFrameLoaders;
- xConfig = impl_openConfig(rGuard, E_PROVIDER_OTHERS);
+ xConfig = impl_openConfig(E_PROVIDER_OTHERS);
sSet = CFGSET_FRAMELOADERS;
}
break;
@@ -1671,7 +1689,7 @@ CacheItemList::iterator
FilterCache::impl_loadItemOnDemand( std::unique_lock<std
case E_CONTENTHANDLER :
{
pList = &m_lContentHandlers;
- xConfig = impl_openConfig(rGuard, E_PROVIDER_OTHERS);
+ xConfig = impl_openConfig(E_PROVIDER_OTHERS);
sSet = CFGSET_CONTENTHANDLERS;
}
break;
@@ -1689,7 +1707,7 @@ CacheItemList::iterator
FilterCache::impl_loadItemOnDemand( std::unique_lock<std
if (bItemInConfig)
{
- (*pList)[sItem] = impl_loadItem(rGuard, xSet, eType, sItem,
E_READ_ALL);
+ (*pList)[sItem] = impl_loadItem(xSet, eType, sItem, E_READ_ALL);
}
else
{
@@ -1951,14 +1969,14 @@ void FilterCache::impl_interpretDataVal4Filter(const
OUString& sValue,
TODO work on a cache copy first, which can be flushed afterwards
That would be useful to guarantee a consistent cache.
-----------------------------------------------*/
-void FilterCache::impl_readOldFormat(std::unique_lock<std::mutex>& rGuard)
+void FilterCache::impl_readOldFormat()
{
// Attention: Opening/Reading of this old configuration format has to be
handled gracefully.
// It's optional and should not disturb our normal work!
// E.g. we must check, if the package exists...
try
{
- css::uno::Reference< css::uno::XInterface > xInt =
impl_openConfig(rGuard, E_PROVIDER_OLD);
+ css::uno::Reference< css::uno::XInterface > xInt =
impl_openConfig(E_PROVIDER_OLD);
css::uno::Reference< css::container::XNameAccess > xCfg(xInt,
css::uno::UNO_QUERY_THROW);
OUString TYPES_SET("Types");
@@ -1970,7 +1988,7 @@ void
FilterCache::impl_readOldFormat(std::unique_lock<std::mutex>& rGuard)
xCfg->getByName(TYPES_SET) >>= xSet;
const css::uno::Sequence< OUString > lItems =
xSet->getElementNames();
for (const OUString& rName : lItems)
- m_lTypes[rName] = impl_readOldItem(rGuard, xSet, E_TYPE,
rName);
+ m_lTypes[rName] = impl_readOldItem(xSet, E_TYPE, rName);
}
OUString FILTER_SET("Filters");
@@ -1981,7 +1999,7 @@ void
FilterCache::impl_readOldFormat(std::unique_lock<std::mutex>& rGuard)
xCfg->getByName(FILTER_SET) >>= xSet;
const css::uno::Sequence< OUString > lItems =
xSet->getElementNames();
for (const OUString& rName : lItems)
- m_lFilters[rName] = impl_readOldItem(rGuard, xSet, E_FILTER,
rName);
+ m_lFilters[rName] = impl_readOldItem(xSet, E_FILTER, rName);
}
}
/* corrupt filter addon? Because it's external (optional) code... we can
ignore it. Addon won't work then...
@@ -1993,8 +2011,7 @@ void
FilterCache::impl_readOldFormat(std::unique_lock<std::mutex>& rGuard)
}
}
-CacheItem FilterCache::impl_readOldItem(std::unique_lock<std::mutex>& rGuard,
- const css::uno::Reference<
css::container::XNameAccess >& xSet ,
+CacheItem FilterCache::impl_readOldItem(const css::uno::Reference<
css::container::XNameAccess >& xSet ,
EItemType
eType,
const OUString&
sItem)
{
@@ -2010,7 +2027,7 @@ CacheItem
FilterCache::impl_readOldItem(std::unique_lock<std::mutex>& rGuard,
// Isn't used any longer!
// UIName
- impl_readPatchUINames(rGuard, xItem, aItem);
+ impl_readPatchUINames(xItem, aItem);
// Data
OUString sData;
@@ -2097,16 +2114,21 @@ OUString
FilterCache::impl_searchContentHandlerForType(const OUString& sType) co
#endif
-bool FilterCache::impl_isModuleInstalled(std::unique_lock<std::mutex>&
/*rGuard*/, const OUString& sModule)
+bool FilterCache::impl_isModuleInstalled(const OUString& sModule)
{
css::uno::Reference< css::container::XNameAccess > xCfg;
- if (!m_xModuleCfg.is())
+ // SAFE ->
{
- m_xModuleCfg = officecfg::Setup::Office::Factories::get();
- }
+ osl::MutexGuard aLock(m_aMutex);
+ if (!m_xModuleCfg.is())
+ {
+ m_xModuleCfg = officecfg::Setup::Office::Factories::get();
+ }
- xCfg = m_xModuleCfg;
+ xCfg = m_xModuleCfg;
+ }
+ // <- SAFE
if (xCfg.is())
return xCfg->hasByName(sModule);
diff --git a/filter/source/config/cache/filtercache.hxx
b/filter/source/config/cache/filtercache.hxx
index abfa712895d1..8445efea8227 100644
--- a/filter/source/config/cache/filtercache.hxx
+++ b/filter/source/config/cache/filtercache.hxx
@@ -20,7 +20,6 @@
#pragma once
#include <memory>
-#include <mutex>
#include "cacheitem.hxx"
#include <com/sun/star/beans/NamedValue.hpp>
@@ -57,7 +56,7 @@ class CacheUpdateListener;
Further we make it public. So any user of this class
can lock us from outside too.
*/
-class FilterCache
+class FilterCache : public cppu::BaseMutex
{
// public types
@@ -162,7 +161,6 @@ class FilterCache
private:
- mutable std::mutex m_aMutex;
/** @short holds the used configuration provider alive, which
provides access to the list of types. */
@@ -583,7 +581,6 @@ class FilterCache
private:
- std::vector<OUString> getItemNames(std::unique_lock<std::mutex>&
rGuard, EItemType eType) const;
/** @short return a reference to one of our internal
sub container, which contains items of the
@@ -598,11 +595,11 @@ class FilterCache
@throw [css::uno::Exception]
if the required list does not exist.
*/
- const CacheItemList& impl_getItemList(std::unique_lock<std::mutex>&
rGuard, EItemType eType) const;
+ const CacheItemList& impl_getItemList(EItemType eType) const;
- CacheItemList& impl_getItemList(std::unique_lock<std::mutex>& rGuard,
EItemType eType);
+ CacheItemList& impl_getItemList(EItemType eType);
- CacheItem& impl_getItem( std::unique_lock<std::mutex>& rGuard,
EItemType eType, const OUString& sItem);
+ CacheItem& impl_getItem( EItemType eType, const OUString& sItem);
/** @short return a valid configuration update access
to the underlying configuration package, which
@@ -623,7 +620,7 @@ class FilterCache
all necessary listener connections will be established
too. So this cache will be informed about outside
updates.
*/
- css::uno::Reference< css::uno::XInterface >
impl_openConfig(std::unique_lock<std::mutex>& rGuard, EConfigProvider eProvide);
+ css::uno::Reference< css::uno::XInterface >
impl_openConfig(EConfigProvider eProvide);
/** @short tries to open the requested configuration root
@@ -644,8 +641,7 @@ class FilterCache
and initialized within the requested modes
successfully;
a NULL reference otherwise.
*/
- static css::uno::Reference< css::uno::XInterface >
impl_createConfigAccess(std::unique_lock<std::mutex>& rGuard,
-
const OUString& sRoot ,
+ css::uno::Reference< css::uno::XInterface >
impl_createConfigAccess(const OUString& sRoot ,
bool bReadOnly ,
bool bLocalesMode);
@@ -669,7 +665,7 @@ class FilterCache
Can be empty if an internal error occurred or if the
requested
key does not exists!
*/
- static css::uno::Any
impl_getDirectCFGValue(std::unique_lock<std::mutex>& rGuard,
std::u16string_view sDirectKey);
+ css::uno::Any impl_getDirectCFGValue(std::u16string_view sDirectKey);
/** @short load the underlying configuration into this cache.
@@ -684,7 +680,7 @@ class FilterCache
@throws css::uno::Exception
*/
- void impl_load(std::unique_lock<std::mutex>& rGuard, EFillState
eRequiredState);
+ void impl_load(EFillState eRequiredState);
/** @short validate the whole cache and create
@@ -703,7 +699,7 @@ class FilterCache
@throw [css::uno::Exception]
if cache is invalid and could not be repaired.
*/
- void impl_validateAndOptimize(std::unique_lock<std::mutex>& rGuard);
+ void impl_validateAndOptimize();
private:
@@ -733,8 +729,7 @@ class FilterCache
@throw [css::uno::Exception]
if an unrecoverable error occurs inside this operation.
*/
- void impl_loadSet(std::unique_lock<std::mutex>& rGuard,
- const css::uno::Reference<
css::container::XNameAccess >& xConfig,
+ void impl_loadSet(const css::uno::Reference<
css::container::XNameAccess >& xConfig,
EItemType
eType ,
EReadOption
eOption,
CacheItemList*
pCache );
@@ -762,8 +757,7 @@ class FilterCache
@throw [css::uno::Exception]
if an unrecoverable error occurs inside this operation.
*/
- CacheItem impl_loadItem(std::unique_lock<std::mutex>& rGuard,
- const css::uno::Reference<
css::container::XNameAccess >& xSet ,
+ CacheItem impl_loadItem(const css::uno::Reference<
css::container::XNameAccess >& xSet ,
EItemType
eType ,
const OUString&
sItem ,
EReadOption
eOption);
@@ -794,8 +788,7 @@ class FilterCache
@throw [css::uno::Exception]
if an unrecoverable error occurs inside this operation.
*/
- CacheItemList::iterator impl_loadItemOnDemand(
std::unique_lock<std::mutex>& rGuard,
- EItemType eType,
+ CacheItemList::iterator impl_loadItemOnDemand( EItemType
eType,
const OUString& sItem);
@@ -862,8 +855,7 @@ class FilterCache
@throws css::uno::Exception
*/
- void impl_readPatchUINames(std::unique_lock<std::mutex>& rGuard,
- const css::uno::Reference<
css::container::XNameAccess >& xNode,
+ void impl_readPatchUINames(const css::uno::Reference<
css::container::XNameAccess >& xNode,
CacheItem&
rItem);
@@ -875,14 +867,13 @@ class FilterCache
const CacheItem&
rItem);
/** TODO */
- void impl_readOldFormat(std::unique_lock<std::mutex>& rGuard);
+ void impl_readOldFormat();
/** TODO
@throws css::uno::Exception
*/
- CacheItem impl_readOldItem(std::unique_lock<std::mutex>& rGuard,
- const css::uno::Reference<
css::container::XNameAccess >& xSet ,
+ CacheItem impl_readOldItem(const css::uno::Reference<
css::container::XNameAccess >& xSet ,
EItemType
eType,
const OUString&
sItem);
@@ -918,7 +909,7 @@ class FilterCache
@return sal_True if the requested module is installed; sal_False
otherwise.
*/
- bool impl_isModuleInstalled(std::unique_lock<std::mutex>& rGuard,
const OUString& sModule);
+ bool impl_isModuleInstalled(const OUString& sModule);
/** @short convert a list of flag names to its int representation.
