[Libreoffice-commits] core.git: cppuhelper/source
Stephan Bergmann (via logerrit)
logerrit at kemper.freedesktop.org
Thu Oct 1 07:45:37 UTC 2020
cppuhelper/source/servicemanager.cxx | 97 +++++++++++++++++++++++++----------
cppuhelper/source/servicemanager.hxx | 19 +++++-
2 files changed, 88 insertions(+), 28 deletions(-)
New commits:
commit 1a1d432420646bcf129624a866fd6c754e25d195
Author: Stephan Bergmann <sbergman at redhat.com>
AuthorDate: Wed Sep 30 21:31:10 2020 +0200
Commit: Stephan Bergmann <sbergman at redhat.com>
CommitDate: Thu Oct 1 09:44:53 2020 +0200
Add single-instance attribute for *.compoonent <implementation>s
...to ease the implementation in C++ code of constructor-based <implementation>s
that shall be single-instance (and which would have been implemented with e.g.
cppu::creaetOneInstanceFactory for non--constructor-based <implementation>s).
See e.g. 6e35794cad555485955c3b43593497dcdbf29840 "terminate XDesktop properly
in unit tests" and 6362ebab298549e8616c32cafd75cb3959ba7d65 "dbaccess: create
instances with uno constructors" for the clumsy approach used until now, where
the C++ constructor function uses a static instance that is cleared in
dispose(), adding fake <singleton> entries to <implementation>s where necessary
so that the ServiceManager will call those XComponent::dispose() functions when
it itself gets disposed.
For every <implementation>, the ServiceManager already holds an Implementation
data structure, so it can easily hold a singleInstance there and clear it when
the ServiceManager gets disposed. (One consequence is that single-instance
implementations are now created with their Instance.mutex locked, but that
should not cause problems in practice, given that the construction of a single-
instance implementation should not recursively request its own construction
anyway.)
The new single-instance="true" attribute is mostly useful in combination with
the constructor attribute (see above), but it can also be used for non--
constructor-based <implementation>s, at least in theory.
(The single-instance="true" attribute is orthogonal to <singleton> elements.
There are existing single-instance services---even if those should arguably have
been defined as singletons in the first place---that can benefit from
single-instance="true". And there are <implementation>s that support one or
more <singleton>s alongside one or more <service>s, where the latter are not
single-instance.)
This new single-instance="true" attribute in *.component files should not
interfere with the lo_get_constructor_map machinery in the DISABLE_DYNLOADING
branch of cppuhelper::detail::loadSharedLibComponentFactory
(cppuhelper/source/shlib.cxx) used by Android, iOS and some fuzzer code.
AFAIU, the lo_get_constructor_map machinery should only ever come into play
after the ServiceManager has decided whether or not to create a new instance
based on the single-instance attributes in the *.component files.
This commit only provides the new single-instance="true" attribute. Actual
changes of <implementation>s and their C++ constructor functions are delegated
to a series of follow-up commits. (So that they can easily be reverted
individually should the need arise.)
Change-Id: Iea6c0fc539d74477b7a536dc771b198df6b0510e
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/103734
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <sbergman at redhat.com>
diff --git a/cppuhelper/source/servicemanager.cxx b/cppuhelper/source/servicemanager.cxx
index f06405f7794b..fe9ef7dbd2b4 100644
--- a/cppuhelper/source/servicemanager.cxx
+++ b/cppuhelper/source/servicemanager.cxx
@@ -327,6 +327,7 @@ void Parser::handleComponent() {
void Parser::handleImplementation() {
OUString attrName;
OUString attrConstructor;
+ bool attrSingleInstance = false;
xmlreader::Span name;
int nsId;
while (reader_.nextAttribute(&nsId, &name)) {
@@ -366,6 +367,19 @@ void Parser::handleImplementation() {
+ ": <implementation> has \"constructor\" attribute but"
" <component> has no \"environment\" attribute");
}
+ } else if (nsId == xmlreader::XmlReader::NAMESPACE_NONE
+ && name.equals(RTL_CONSTASCII_STRINGPARAM("single-instance")))
+ {
+ if (attrSingleInstance) {
+ throw css::registry::InvalidRegistryException(
+ reader_.getUrl()
+ + ": <implementation> has multiple \"single-instance\" attributes");
+ }
+ if (!reader_.getAttributeValue(false).equals(RTL_CONSTASCII_STRINGPARAM("true"))) {
+ throw css::registry::InvalidRegistryException(
+ reader_.getUrl() + ": <implementation> has bad \"single-instance\" attribute");
+ }
+ attrSingleInstance = true;
} else {
throw css::registry::InvalidRegistryException(
reader_.getUrl() + ": unexpected element attribute \""
@@ -380,7 +394,7 @@ void Parser::handleImplementation() {
implementation_ =
std::make_shared<cppuhelper::ServiceManager::Data::Implementation>(
attrName, attrLoader_, attrUri_, attrEnvironment_, attrConstructor,
- attrPrefix_, alienContext_, reader_.getUrl());
+ attrPrefix_, attrSingleInstance, alienContext_, reader_.getUrl());
if (!data_->namedImplementations.emplace(attrName, implementation_).
second)
{
@@ -647,28 +661,62 @@ cppuhelper::ServiceManager::Data::Implementation::createInstance(
bool singletonRequest)
{
css::uno::Reference<css::uno::XInterface> inst;
+ if (isSingleInstance) {
+ osl::MutexGuard g(mutex);
+ if (!singleInstance.is()) {
+ singleInstance = doCreateInstance(context);
+ }
+ inst = singleInstance;
+ } else {
+ inst = doCreateInstance(context);
+ }
+ updateDisposeInstance(singletonRequest, inst);
+ return inst;
+}
+
+css::uno::Reference<css::uno::XInterface>
+cppuhelper::ServiceManager::Data::Implementation::createInstanceWithArguments(
+ css::uno::Reference<css::uno::XComponentContext> const & context,
+ bool singletonRequest, css::uno::Sequence<css::uno::Any> const & arguments)
+{
+ css::uno::Reference<css::uno::XInterface> inst;
+ if (isSingleInstance) {
+ osl::MutexGuard g(mutex);
+ if (!singleInstance.is()) {
+ singleInstance = doCreateInstanceWithArguments(context, arguments);
+ }
+ inst = singleInstance;
+ } else {
+ inst = doCreateInstanceWithArguments(context, arguments);
+ }
+ updateDisposeInstance(singletonRequest, inst);
+ return inst;
+}
+
+css::uno::Reference<css::uno::XInterface>
+cppuhelper::ServiceManager::Data::Implementation::doCreateInstance(
+ css::uno::Reference<css::uno::XComponentContext> const & context)
+{
if (constructorFn) {
- inst.set(
+ return css::uno::Reference<css::uno::XInterface>(
constructorFn(context.get(), css::uno::Sequence<css::uno::Any>()),
SAL_NO_ACQUIRE);
} else if (factory1.is()) {
- inst = factory1->createInstanceWithContext(context);
+ return factory1->createInstanceWithContext(context);
} else {
assert(factory2.is());
- inst = factory2->createInstance();
+ return factory2->createInstance();
}
- updateDisposeSingleton(singletonRequest, inst);
- return inst;
}
css::uno::Reference<css::uno::XInterface>
-cppuhelper::ServiceManager::Data::Implementation::createInstanceWithArguments(
+cppuhelper::ServiceManager::Data::Implementation::doCreateInstanceWithArguments(
css::uno::Reference<css::uno::XComponentContext> const & context,
- bool singletonRequest, css::uno::Sequence<css::uno::Any> const & arguments)
+ css::uno::Sequence<css::uno::Any> const & arguments)
{
- css::uno::Reference<css::uno::XInterface> inst;
if (constructorFn) {
- inst.set(constructorFn(context.get(), arguments), SAL_NO_ACQUIRE);
+ css::uno::Reference<css::uno::XInterface> inst(
+ constructorFn(context.get(), arguments), SAL_NO_ACQUIRE);
//HACK: The constructor will either observe arguments and return inst
// that does not implement XInitialization (or null), or ignore
// arguments and return inst that implements XInitialization; this
@@ -679,18 +727,17 @@ cppuhelper::ServiceManager::Data::Implementation::createInstanceWithArguments(
if (init.is()) {
init->initialize(arguments);
}
+ return inst;
} else if (factory1.is()) {
- inst = factory1->createInstanceWithArgumentsAndContext(
+ return factory1->createInstanceWithArgumentsAndContext(
arguments, context);
} else {
assert(factory2.is());
- inst = factory2->createInstanceWithArguments(arguments);
+ return factory2->createInstanceWithArguments(arguments);
}
- updateDisposeSingleton(singletonRequest, inst);
- return inst;
}
-void cppuhelper::ServiceManager::Data::Implementation::updateDisposeSingleton(
+void cppuhelper::ServiceManager::Data::Implementation::updateDisposeInstance(
bool singletonRequest,
css::uno::Reference<css::uno::XInterface> const & instance)
{
@@ -702,15 +749,15 @@ void cppuhelper::ServiceManager::Data::Implementation::updateDisposeSingleton(
// the implementation hands out different instances):
if (singletonRequest) {
osl::MutexGuard g(mutex);
- disposeSingleton.clear();
+ disposeInstance.clear();
dispose = false;
- } else if (!singletons.empty()) {
+ } else if (shallDispose()) {
css::uno::Reference<css::lang::XComponent> comp(
instance, css::uno::UNO_QUERY);
if (comp.is()) {
osl::MutexGuard g(mutex);
if (dispose) {
- disposeSingleton = comp;
+ disposeInstance = comp;
}
}
}
@@ -840,20 +887,20 @@ void cppuhelper::ServiceManager::disposing() {
for (const auto& rEntry : data_.namedImplementations)
{
assert(rEntry.second);
- if (!rEntry.second->singletons.empty()) {
+ if (rEntry.second->shallDispose()) {
osl::MutexGuard g2(rEntry.second->mutex);
- if (rEntry.second->disposeSingleton.is()) {
- sngls.push_back(rEntry.second->disposeSingleton);
+ if (rEntry.second->disposeInstance.is()) {
+ sngls.push_back(rEntry.second->disposeInstance);
}
}
}
for (const auto& rEntry : data_.dynamicImplementations)
{
assert(rEntry.second);
- if (!rEntry.second->singletons.empty()) {
+ if (rEntry.second->shallDispose()) {
osl::MutexGuard g2(rEntry.second->mutex);
- if (rEntry.second->disposeSingleton.is()) {
- sngls.push_back(rEntry.second->disposeSingleton);
+ if (rEntry.second->disposeInstance.is()) {
+ sngls.push_back(rEntry.second->disposeInstance);
}
}
if (rEntry.second->component.is()) {
@@ -1395,7 +1442,7 @@ bool cppuhelper::ServiceManager::readLegacyRdbFile(OUString const & uri) {
std::shared_ptr< Data::Implementation > impl =
std::make_shared<Data::Implementation>(
name, readLegacyRdbString(uri, implKey, "UNO/ACTIVATOR"),
- readLegacyRdbString(uri, implKey, "UNO/LOCATION"), "", "", "",
+ readLegacyRdbString(uri, implKey, "UNO/LOCATION"), "", "", "", false,
css::uno::Reference< css::uno::XComponentContext >(), uri);
if (!data_.namedImplementations.emplace(name, impl).second)
{
diff --git a/cppuhelper/source/servicemanager.hxx b/cppuhelper/source/servicemanager.hxx
index 9a7cac513402..24a7e56a4ba9 100644
--- a/cppuhelper/source/servicemanager.hxx
+++ b/cppuhelper/source/servicemanager.hxx
@@ -74,11 +74,13 @@ public:
OUString const & theUri, OUString const & theEnvironment,
OUString const & theConstructorName,
OUString const & thePrefix,
+ bool theIsSingleInstance,
css::uno::Reference< css::uno::XComponentContext > const &
theAlienContext,
OUString const & theRdbFile):
name(theName), loader(theLoader), uri(theUri), environment(theEnvironment),
constructorName(theConstructorName), prefix(thePrefix),
+ isSingleInstance(theIsSingleInstance),
alienContext(theAlienContext), rdbFile(theRdbFile),
constructorFn(nullptr), status(STATUS_NEW), dispose(true)
{}
@@ -91,7 +93,7 @@ public:
theFactory2,
css::uno::Reference< css::lang::XComponent > const &
theComponent):
- name(theName), constructorFn(nullptr),
+ name(theName), isSingleInstance(false), constructorFn(nullptr),
factory1(theFactory1), factory2(theFactory2),
component(theComponent), status(STATUS_LOADED), dispose(true)
{ assert(theFactory1.is() || theFactory2.is()); }
@@ -111,6 +113,8 @@ public:
bool singletonRequest,
css::uno::Sequence<css::uno::Any> const & arguments);
+ bool shallDispose() const { return isSingleInstance || !singletons.empty(); }
+
enum Status { STATUS_NEW, STATUS_WRAPPER, STATUS_LOADED };
// Logically, exactly one of constructorFn, factory1, factory2 should
@@ -130,6 +134,7 @@ public:
OUString environment;
OUString constructorName;
OUString prefix;
+ bool isSingleInstance;
css::uno::Reference< css::uno::XComponentContext > alienContext;
OUString rdbFile;
std::vector< OUString > services;
@@ -141,11 +146,19 @@ public:
Status status;
osl::Mutex mutex;
- css::uno::Reference< css::lang::XComponent > disposeSingleton;
+ css::uno::Reference<css::uno::XInterface> singleInstance;
+ css::uno::Reference< css::lang::XComponent > disposeInstance;
bool dispose;
private:
- void updateDisposeSingleton(
+ css::uno::Reference<css::uno::XInterface> doCreateInstance(
+ css::uno::Reference<css::uno::XComponentContext> const & context);
+
+ css::uno::Reference<css::uno::XInterface> doCreateInstanceWithArguments(
+ css::uno::Reference<css::uno::XComponentContext> const & context,
+ css::uno::Sequence<css::uno::Any> const & arguments);
+
+ void updateDisposeInstance(
bool singletonRequest,
css::uno::Reference<css::uno::XInterface> const & instance);
};
More information about the Libreoffice-commits
mailing list