static/emscripten/uno.js | 17 +++++++--------
static/source/embindmaker/embindmaker.cxx | 32 ++++++++++++++++++++++++------
unotest/source/embindtest/embindtest.js | 13 ++++++++++++
3 files changed, 47 insertions(+), 15 deletions(-)
New commits:
commit 04658a706757dabbedfd87717e6f1f354b4c8961
Author: Stephan Bergmann <stephan.bergm...@allotropia.de>
AuthorDate: Mon Jun 17 15:54:43 2024 +0200
Commit: Stephan Bergmann <stephan.bergm...@allotropia.de>
CommitDate: Mon Jun 17 21:57:12 2024 +0200
Embind: Fix lifecycle of UNO any and sequence values returned from JS to C++
When a JS function implementing a UNO interface method returns any or a
sequence
type (like queryInterface, getType, and getImplementationId in uno.js), it
could not create a new instance of that type and return it, as it would have
needed to call .delete() on that instance, but couldn't. In uno.js,
getType and
getImplementationId solved that by returning pre-instantiated instances that
were deleted in the final release call. But that did not work for
queryInterface, as pre-instantiating the relevant any instances would have
caused cyclic references that would have caused the final release call
never to
occur.
So redesign the C++ the_wrapper classes (used by the Embind allow_subclass
machinery): If a UNO interface method returns any or a sequence type
(i.e., a
type on which .delete() must be called), change the JS implemenation's
return
type from by-value (which meant that the C++ code received a copy) to
by-reference---which means that now the C++ code can access the original
instance and delete it. But which also means that the JS code must always
return a fresh instance now!
(Ideally, the embindmaker-generated code would use by-pointer rather than
by-reference for that return type, but that caused
> emsdk/upstream/emscripten/cache/sysroot/include/emscripten/wire.h:116:19:
error: static assertion failed due to requirement
'!std::is_pointer<com::sun::star::uno::Any *>::value': Implicitly binding raw
pointers is illegal. Specify allow_raw_pointer<arg<?>>
> 116 | static_assert(!std::is_pointer<T*>::value, "Implicitly
binding raw pointers is illegal. Specify allow_raw_pointer<arg<?>>");
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~
errors with no obvious place where to put such allow_raw_pointer markers, so
lets go with this little hack at least for now.)
Change-Id: I3c37b79b8fbf09c19782c6532bc95d4d63505c63
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/169008
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <stephan.bergm...@allotropia.de>
diff --git a/static/emscripten/uno.js b/static/emscripten/uno.js
index fd3b77192d03..2398dee76da7 100644
--- a/static/emscripten/uno.js
+++ b/static/emscripten/uno.js
@@ -17,11 +17,6 @@ Module.unoObject = function(interfaces, obj) {
Module.initUno();
interfaces = ['com.sun.star.lang.XTypeProvider'].concat(interfaces);
obj.impl_refcount = 0;
- obj.impl_types = new Module.uno_Sequence_type(interfaces.length,
Module.uno_Sequence.FromSize);
- for (let i = 0; i !== interfaces.length; ++i) {
- obj.impl_types.set(i, Module.uno_Type.Interface(interfaces[i]));
- }
- obj.impl_implementationId = new Module.uno_Sequence_byte([]);
obj.queryInterface = function(type) {
for (const i in obj.impl_typemap) {
if (i === type.toString()) {
@@ -39,12 +34,16 @@ Module.unoObject = function(interfaces, obj) {
for (const i in obj.impl_interfaces) {
obj.impl_interfaces[i].delete();
}
- obj.impl_types.delete();
- obj.impl_implementationId.delete();
}
};
- obj.getTypes = function() { return obj.impl_types; };
- obj.getImplementationId = function() { return obj.impl_implementationId; };
+ obj.getTypes = function() {
+ const types = new Module.uno_Sequence_type(interfaces.length,
Module.uno_Sequence.FromSize);
+ for (let i = 0; i !== interfaces.length; ++i) {
+ types.set(i, Module.uno_Type.Interface(interfaces[i]));
+ }
+ return types;
+ };
+ obj.getImplementationId = function() { return new
Module.uno_Sequence_byte([]) };
obj.impl_interfaces = {};
interfaces.forEach((i) => {
obj.impl_interfaces[i] = Module['uno_Type_' + i.replace(/\./g,
'$')].implement(obj);
diff --git a/static/source/embindmaker/embindmaker.cxx
b/static/source/embindmaker/embindmaker.cxx
index b8184a03e0a9..c40b8db6fa24 100644
--- a/static/source/embindmaker/embindmaker.cxx
+++ b/static/source/embindmaker/embindmaker.cxx
@@ -795,14 +795,34 @@ void dumpWrapperClassMembers(std::ostream& out,
rtl::Reference<TypeManager> cons
}
out << " " << param.name;
}
- out << ") override { return call<";
- dumpType(out, manager, meth.returnType);
- out << ">(\"" << meth.name << "\"";
- for (auto const& param : meth.parameters)
+ out << ") override {";
+ if (meth.returnType == "any" || meth.returnType.startsWith("[]"))
{
- out << ", " << param.name;
+ out << "
"
+ " auto & the_ptr = call<";
+ dumpType(out, manager, meth.returnType);
+ out << " const &>(\"" << meth.name << "\"";
+ for (auto const& param : meth.parameters)
+ {
+ out << ", " << param.name;
+ }
+ out << ");
"
+ " auto const the_copy(the_ptr);
"
+ " delete &the_ptr;
"
+ " return the_copy;
"
+ " }
";
+ }
+ else
+ {
+ out << " return call<";
+ dumpType(out, manager, meth.returnType);
+ out << ">(\"" << meth.name << "\"";
+ for (auto const& param : meth.parameters)
+ {
+ out << ", " << param.name;
+ }
+ out << "); }
";
}
- out << "); }
";
}
}
diff --git a/unotest/source/embindtest/embindtest.js
b/unotest/source/embindtest/embindtest.js
index 37a83fba9f4e..355463844da8 100644
--- a/unotest/source/embindtest/embindtest.js
+++ b/unotest/source/embindtest/embindtest.js
@@ -658,6 +658,19 @@ Module.addOnPostRun(function() {
},
trigger(event) { console.log('Ola ' + event); }
});
+ {
+ const s = css.lang.XTypeProvider.query(obj).getTypes();
+ console.assert(s.size() === 3);
+ console.assert(s.get(0).toString() ===
'com.sun.star.lang.XTypeProvider');
+ console.assert(s.get(1).toString() === 'com.sun.star.task.XJob');
+ console.assert(s.get(2).toString() ===
'com.sun.star.task.XJobExecutor');
+ s.delete();
+ }
+ {
+ const s = css.lang.XTypeProvider.query(obj).getImplementationId();
+ console.assert(s.size() === 0);
+ s.delete();
+ }
test.passJob(css.task.XJob.query(obj));
test.passJobExecutor(css.task.XJobExecutor.query(obj));
test.passInterface(obj);
