LibreOffice/core
1
0
Fork 0
mirror of https://github.com/LibreOffice/core.git synced 2025-08-02 04:50:01 +00:00
Code Issues Packages Projects Releases Wiki Activity

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

Browse Source 
This reverts commit 3a4a00a51a.

<x1sc0> noelgrandin, regarding tdf#157042, the commit was reverted in 24-2 and 7-6 branches but not in master so I was wondering what to do next. There are clear steps on how to reproduce it in comment 27 but it seems the crash is not reproducible with a debug build ( according to comment 37 )
<noelgrandin> x1sc0, let me try to reproduce that
<noelgrandin> x1sc0, I cant reproduce, please just revert that on master

Change-Id: I45dcf8f4b422e1a19eaa41ec7614db569b5aac7c
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/163125
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
Tested-by: Jenkins
Reviewed-by: Xisco Fauli <xiscofauli@libreoffice.org>
This commit is contained in:
Xisco Fauli
2024-02-08 17:10:03 +01:00
parent c2c651ee22
commit cfcd6b2557
2 changed files with 31 additions and 97 deletions
Download Patch File Download Diff File Expand all files Collapse all files

106
comphelper/source/misc/configuration.cxx
View File

@ -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>
@ -109,68 +108,13 @@ 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(
css::uno::Reference<css::uno::XComponentContext> const & context):
context_(context.is() ? 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_ ) );
access_(css::configuration::ReadWriteAccess::create(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);
}
}
comphelper::detail::ConfigurationWrapper::~ConfigurationWrapper()
{
maPropertyCache.clear();
maNotifier.clear();
maListener.clear();
}
comphelper::detail::ConfigurationWrapper::~ConfigurationWrapper() {}
bool comphelper::detail::ConfigurationWrapper::isReadOnly(OUString const & path)
const
@ -181,30 +125,34 @@ 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();
// should be short-circuited in ConfigurationProperty::get()
assert(!comphelper::IsFuzzing());
// 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);
OUString parentPath(path.substr(0, idx));
OUString childName(path.substr(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;
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(

22
include/comphelper/configuration.hxx
View File

@ -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 {
@ -87,18 +82,15 @@ private:
namespace detail {
class ConfigurationChangesListener;
/// @internal
class COMPHELPER_DLLPUBLIC ConfigurationWrapper {
friend class ConfigurationChangesListener;
public:
static ConfigurationWrapper const & get(
css::uno::Reference<css::uno::XComponentContext> const & context);
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,
@ -147,12 +139,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