comphelper/source/misc/configuration.cxx | 59 ++++++++++++++-----------------
include/comphelper/configuration.hxx | 20 ++++++++--
2 files changed, 44 insertions(+), 35 deletions(-)
New commits:
commit 203ad037ccb9fdebffea4f622229ded90635eb8b
Author: Noel Grandin <noelgran...@gmail.com>
AuthorDate: Tue Dec 27 16:25:25 2022 +0200
Commit: Noel Grandin <noel.gran...@collabora.co.uk>
CommitDate: Wed Dec 28 12:01:55 2022 +0000
try to fix shutdown crashes in ConfigurationWrapper
associated with
commit 7df433cdc33b4d6ba38eafad9282d015571433ef
optimize ConfigurationProperty::get()
it seems it introduced
https://crashreport.libreoffice.org/stats/signature/void%20rtl::str::release%3C_rtl_uString%3E(_rtl_uString*)
which, at the moment, is the most reported crash in
https://crashreport.libreoffice.org/stats/version/7.4.3.2
We need to tie the lifetime of ConfigurationWrapper and the cache
objects together, so they die as one unit, otherwise the
unpredicatable ordering of destruction of static objects is a
problem.
Change-Id: I80db5a4c28510a68f40b919902b01f3981e32bfb
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/144840
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.gran...@collabora.co.uk>
(cherry picked from commit 1b9ca3689dab4deaea20be511eb942674bbbc481)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/144807
diff --git a/comphelper/source/misc/configuration.cxx
b/comphelper/source/misc/configuration.cxx
index 1d6517fbb26d..b84f705f1db0 100644
--- a/comphelper/source/misc/configuration.cxx
+++ b/comphelper/source/misc/configuration.cxx
@@ -10,9 +10,6 @@
#include <sal/config.h>
#include <cassert>
-#include <mutex>
-#include <string_view>
-#include <unordered_map>
#include <com/sun/star/beans/NamedValue.hpp>
#include <com/sun/star/beans/PropertyAttribute.hpp>
@@ -24,11 +21,12 @@
#include <com/sun/star/container/XHierarchicalNameReplace.hpp>
#include <com/sun/star/container/XNameAccess.hpp>
#include <com/sun/star/container/XNameContainer.hpp>
+#include <com/sun/star/util/XChangesListener.hpp>
+#include <com/sun/star/util/XChangesNotifier.hpp>
+#include <com/sun/star/lang/DisposedException.hpp>
#include <com/sun/star/lang/XLocalizable.hpp>
#include <com/sun/star/uno/Any.hxx>
#include <com/sun/star/uno/Reference.hxx>
-#include <com/sun/star/util/XChangesListener.hpp>
-#include <com/sun/star/util/XChangesNotifier.hpp>
#include <comphelper/solarmutex.hxx>
#include <comphelper/configuration.hxx>
#include <comphelper/configurationlistener.hxx>
@@ -109,37 +107,32 @@ comphelper::detail::ConfigurationWrapper::get()
return WRAPPER;
}
-namespace
-{
-std::mutex gMutex;
-std::unordered_map<OUString, css::uno::Any> gPropertyCache;
-css::uno::Reference< css::util::XChangesNotifier > gNotifier;
-css::uno::Reference< css::util::XChangesListener > gListener;
-
-class ConfigurationChangesListener
+class comphelper::detail::ConfigurationChangesListener
: public ::cppu::WeakImplHelper<css::util::XChangesListener>
{
+ comphelper::detail::ConfigurationWrapper& mrConfigurationWrapper;
public:
- ConfigurationChangesListener()
+ ConfigurationChangesListener(comphelper::detail::ConfigurationWrapper&
rWrapper)
+ : mrConfigurationWrapper(rWrapper)
{}
// util::XChangesListener
virtual void SAL_CALL changesOccurred( const css::util::ChangesEvent& )
override
{
- std::scoped_lock aGuard(gMutex);
- gPropertyCache.clear();
+ std::scoped_lock aGuard(mrConfigurationWrapper.maMutex);
+ mrConfigurationWrapper.maPropertyCache.clear();
}
virtual void SAL_CALL disposing(const css::lang::EventObject&) override
{
- std::scoped_lock aGuard(gMutex);
- gPropertyCache.clear();
+ std::scoped_lock aGuard(mrConfigurationWrapper.maMutex);
+ mrConfigurationWrapper.mbDisposed = true;
+ mrConfigurationWrapper.maPropertyCache.clear();
}
};
-} // namespace
-
comphelper::detail::ConfigurationWrapper::ConfigurationWrapper():
context_(comphelper::getProcessComponentContext()),
- access_(css::configuration::ReadWriteAccess::create(context_, "*"))
+ access_(css::configuration::ReadWriteAccess::create(context_, "*")),
+ mbDisposed(false)
{
// Set up a configuration notifier to invalidate the cache as needed.
try
@@ -156,10 +149,10 @@
comphelper::detail::ConfigurationWrapper::ConfigurationWrapper():
=
xConfigProvider->createInstanceWithArguments(u"com.sun.star.configuration.ConfigurationAccess",
params);
- gNotifier = css::uno::Reference< css::util::XChangesNotifier >(xCfg,
css::uno::UNO_QUERY);
- assert(gNotifier.is());
- gListener = css::uno::Reference< ConfigurationChangesListener >(new
ConfigurationChangesListener());
- gNotifier->addChangesListener(gListener);
+ maNotifier = css::uno::Reference< css::util::XChangesNotifier >(xCfg,
css::uno::UNO_QUERY);
+ assert(maNotifier.is());
+ maListener = css::uno::Reference< ConfigurationChangesListener >(new
ConfigurationChangesListener(*this));
+ maNotifier->addChangesListener(maListener);
}
catch(const css::uno::Exception&)
{
@@ -169,9 +162,9 @@
comphelper::detail::ConfigurationWrapper::ConfigurationWrapper():
comphelper::detail::ConfigurationWrapper::~ConfigurationWrapper()
{
- gPropertyCache.clear();
- gNotifier.clear();
- gListener.clear();
+ maPropertyCache.clear();
+ maNotifier.clear();
+ maListener.clear();
}
bool comphelper::detail::ConfigurationWrapper::isReadOnly(OUString const &
path)
@@ -185,10 +178,12 @@ bool
comphelper::detail::ConfigurationWrapper::isReadOnly(OUString const & path)
css::uno::Any
comphelper::detail::ConfigurationWrapper::getPropertyValue(OUString const&
path) const
{
- std::scoped_lock aGuard(gMutex);
+ std::scoped_lock aGuard(maMutex);
+ if (mbDisposed)
+ throw css::lang::DisposedException();
// Cache the configuration access, since some of the keys are used in hot
code.
- auto it = gPropertyCache.find(path);
- if( it != gPropertyCache.end())
+ auto it = maPropertyCache.find(path);
+ if( it != maPropertyCache.end())
return it->second;
sal_Int32 idx = path.lastIndexOf("/");
@@ -199,7 +194,7 @@ css::uno::Any
comphelper::detail::ConfigurationWrapper::getPropertyValue(OUStrin
css::uno::Reference<css::container::XNameAccess> access(
access_->getByHierarchicalName(parentPath), css::uno::UNO_QUERY_THROW);
css::uno::Any property = access->getByName(childName);
- gPropertyCache.emplace(path, property);
+ maPropertyCache.emplace(path, property);
return property;
}
diff --git a/include/comphelper/configuration.hxx
b/include/comphelper/configuration.hxx
index 3b8f8f1f7e93..45228b700944 100644
--- a/include/comphelper/configuration.hxx
+++ b/include/comphelper/configuration.hxx
@@ -12,15 +12,16 @@
#include <sal/config.h>
-#include <optional>
-#include <string_view>
-
#include <com/sun/star/uno/Any.hxx>
#include <com/sun/star/uno/Reference.h>
#include <comphelper/comphelperdllapi.h>
#include <comphelper/processfactory.hxx>
#include <sal/types.h>
#include <memory>
+#include <mutex>
+#include <optional>
+#include <string_view>
+#include <unordered_map>
namespace com::sun::star {
namespace configuration { class XReadWriteAccess; }
@@ -31,6 +32,10 @@ namespace com::sun::star {
class XNameContainer;
}
namespace uno { class XComponentContext; }
+ namespace util {
+ class XChangesListener;
+ class XChangesNotifier;
+ }
}
namespace comphelper {
@@ -80,8 +85,11 @@ private:
namespace detail {
+class ConfigurationChangesListener;
+
/// @internal
class COMPHELPER_DLLPUBLIC ConfigurationWrapper {
+friend class ConfigurationChangesListener;
public:
static ConfigurationWrapper const & get();
@@ -135,6 +143,12 @@ private:
// css.beans.XHierarchicalPropertySetInfo), but then
// configmgr::Access::asProperty() would report all properties as
// READONLY, so isReadOnly() would not work
+
+ mutable std::mutex maMutex;
+ bool mbDisposed;
+ mutable std::unordered_map<OUString, css::uno::Any> maPropertyCache;
+ css::uno::Reference< css::util::XChangesNotifier > maNotifier;
+ css::uno::Reference< css::util::XChangesListener > maListener;
};
/// @internal
