tdf#157042: Revert "re-apply "optimize ConfigurationProperty::get()""

This reverts commit 3a4a00a51acca8f9b5e775547abff0c4dc9144d7.

it's causing https://crashreport.libreoffice.org/stats/signature/%3Cname%20omitted%3E
in libreoffice-24-2 branch ( See
https://bugs.documentfoundation.org/show_bug.cgi?id=157042#c36 )
In previous branches, it was reported as
https://crashreport.libreoffice.org/stats/signature/void%20rtl::str::release%3C_rtl_uString%3E(_rtl_uString*)
See comment in 7b46c77366fb3effd2de9bf5ba11ebd3c064974a
"tdf#157042: Revert "re-apply "optimize ConfigurationProperty::get()""

Change-Id: I3481c05b12b422404a38f0be1fea1ea69ffd0e46
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/163061
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>

2 files changed, 32 insertions, 97 deletions

diff --git a/comphelper/source/misc/configuration.cxx b/comphelper/source/misc/configuration.cxx
index 6e500f619232..a724b8654590 100644
--- a/comphelper/source/misc/configuration.cxx
+++ b/comphelper/source/misc/configuration.cxx
@@ -10,8 +10,11 @@
 #include <sal/config.h>
 
 #include <cassert>
+#include <map>
+#include <memory>
+#include <mutex>
+#include <string_view>
 
-#include <com/sun/star/beans/NamedValue.hpp>
 #include <com/sun/star/beans/PropertyAttribute.hpp>
 #include <com/sun/star/configuration/ReadOnlyAccess.hpp>
 #include <com/sun/star/configuration/ReadWriteAccess.hpp>
@@ -21,16 +24,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 <comphelper/solarmutex.hxx>
 #include <comphelper/configuration.hxx>
 #include <comphelper/configurationlistener.hxx>
-#include <cppuhelper/implbase.hxx>
 #include <rtl/ustring.hxx>
 #include <sal/log.hxx>
 #include <i18nlangtag/languagetag.hxx>
@@ -107,67 +106,12 @@ comphelper::detail::ConfigurationWrapper::get()
     return WRAPPER;
 }
 
-class comphelper::detail::ConfigurationChangesListener
-    : public ::cppu::WeakImplHelper<css::util::XChangesListener>
-{
-     comphelper::detail::ConfigurationWrapper& mrConfigurationWrapper;
-public:
-    ConfigurationChangesListener(comphelper::detail::ConfigurationWrapper& rWrapper)
-        : mrConfigurationWrapper(rWrapper)
-    {}
-    // util::XChangesListener
-    virtual void SAL_CALL changesOccurred( const css::util::ChangesEvent& ) override
-    {
-        std::scoped_lock aGuard(mrConfigurationWrapper.maMutex);
-        mrConfigurationWrapper.maPropertyCache.clear();
-    }
-    virtual void SAL_CALL disposing(const css::lang::EventObject&) override
-    {
-        std::scoped_lock aGuard(mrConfigurationWrapper.maMutex);
-        mrConfigurationWrapper.mbDisposed = true;
-        mrConfigurationWrapper.maPropertyCache.clear();
-        mrConfigurationWrapper.maNotifier.clear();
-        mrConfigurationWrapper.maListener.clear();
-    }
-};
-
 comphelper::detail::ConfigurationWrapper::ConfigurationWrapper():
     context_(comphelper::getProcessComponentContext()),
-    access_(css::configuration::ReadWriteAccess::create(context_, "*")),
-    mbDisposed(false)
-{
-    // Set up a configuration notifier to invalidate the cache as needed.
-    try
-    {
-        css::uno::Reference< css::lang::XMultiServiceFactory > xConfigProvider(
-            css::configuration::theDefaultProvider::get( context_ ) );
-
-        // set root path
-        css::uno::Sequence< css::uno::Any > params {
-            css::uno::Any( css::beans::NamedValue{ "nodepath", css::uno::Any( OUString("/"))} ),
-            css::uno::Any( css::beans::NamedValue{ "locale", css::uno::Any( OUString("*"))} ) };
-
-        css::uno::Reference< css::uno::XInterface > xCfg
-            = xConfigProvider->createInstanceWithArguments(u"com.sun.star.configuration.ConfigurationAccess"_ustr,
-                params);
-
-        maNotifier = css::uno::Reference< css::util::XChangesNotifier >(xCfg, css::uno::UNO_QUERY);
-        assert(maNotifier.is());
-        maListener.set(new ConfigurationChangesListener(*this));
-        maNotifier->addChangesListener(maListener);
-    }
-    catch(const css::uno::Exception&)
-    {
-        assert(false);
-    }
-}
+    access_(css::configuration::ReadWriteAccess::create(context_, "*"))
+{}
 
-comphelper::detail::ConfigurationWrapper::~ConfigurationWrapper()
-{
-    maPropertyCache.clear();
-    maNotifier.clear();
-    maListener.clear();
-}
+comphelper::detail::ConfigurationWrapper::~ConfigurationWrapper() {}
 
 bool comphelper::detail::ConfigurationWrapper::isReadOnly(OUString const & path)
     const
@@ -178,26 +122,31 @@ bool comphelper::detail::ConfigurationWrapper::isReadOnly(OUString const & path)
         != 0;
 }
 
-css::uno::Any comphelper::detail::ConfigurationWrapper::getPropertyValue(OUString const& path) const
+css::uno::Any comphelper::detail::ConfigurationWrapper::getPropertyValue(std::u16string_view path) const
 {
-    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 = maPropertyCache.find(path);
-    if( it != maPropertyCache.end())
-        return it->second;
+    // Note that this cache is only used by the officecfg:: auto-generated code, using it for anything
+    // else would be unwise because the cache could end up containing stale entries.
+    static std::mutex gMutex;
+    static std::map<OUString, css::uno::Reference< css::container::XNameAccess >> gAccessMap;
 
-    sal_Int32 idx = path.lastIndexOf("/");
+    sal_Int32 idx = path.rfind('/');
     assert(idx!=-1);
-    OUString parentPath = path.copy(0, idx);
-    OUString childName = path.copy(idx+1);
-
-    css::uno::Reference<css::container::XNameAccess> access(
-        access_->getByHierarchicalName(parentPath), css::uno::UNO_QUERY_THROW);
-    css::uno::Any property = access->getByName(childName);
-    maPropertyCache.emplace(path, property);
-    return property;
+    OUString parentPath(path.substr(0, idx));
+    OUString childName(path.substr(idx+1));
+
+    std::scoped_lock aGuard(gMutex);
+
+    // check cache
+    auto it = gAccessMap.find(parentPath);
+    if (it == gAccessMap.end())
+    {
+        // not in the cache, look it up
+        css::uno::Reference<css::container::XNameAccess> access(
+            access_->getByHierarchicalName(parentPath), css::uno::UNO_QUERY_THROW);
+        it = gAccessMap.emplace(parentPath, access).first;
+    }
+    return it->second->getByName(childName);
 }
 
 void comphelper::detail::ConfigurationWrapper::setPropertyValue(
diff --git a/include/comphelper/configuration.hxx b/include/comphelper/configuration.hxx
index 652e9afaa640..51106a0a12c3 100644
--- a/include/comphelper/configuration.hxx
+++ b/include/comphelper/configuration.hxx
@@ -12,16 +12,15 @@
 
 #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; }
@@ -32,10 +31,6 @@ namespace com::sun::star {
         class XNameContainer;
     }
     namespace uno { class XComponentContext; }
-    namespace util {
-        class XChangesListener;
-        class XChangesNotifier;
-    }
 }
 
 namespace comphelper {
@@ -85,17 +80,14 @@ private:
 
 namespace detail {
 
-class ConfigurationChangesListener;
-
 /// @internal
 class COMPHELPER_DLLPUBLIC ConfigurationWrapper {
-friend class ConfigurationChangesListener;
 public:
     static ConfigurationWrapper const & get();
 
     bool isReadOnly(OUString const & path) const;
 
-    css::uno::Any getPropertyValue(OUString const & path) const;
+    css::uno::Any getPropertyValue(std::u16string_view path) const;
 
     static void setPropertyValue(
         std::shared_ptr< ConfigurationChanges > const & batch,
@@ -143,12 +135,6 @@ 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