-
Notifications
You must be signed in to change notification settings - Fork 3.4k
embind: Add workaround for unstable type-ids between shared libraries and executable #18418
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
embind: Add workaround for unstable type-ids between shared libraries and executable #18418
Conversation
980fb4f
to
c092f82
Compare
2e6b0ce
to
9288d0f
Compare
9288d0f
to
58a8459
Compare
My first impression here is that if you build with visibility=hidden then the types in the side and main module should be considered as separate types. You have created two distinct type universes and types can't be matched with those in the other universe. Does that match with your expectationss or that if this PR? |
When the shared library is built with symbol visibility hidden as the default, the type infos for any type is not available to anything that links to the library, and the library or executable will have its own copy of the type info, which is not merged. The results in emscripten::internal::TypeID having different TYPEID values depending on which module does the lookup, which causes trouble for embind's type registry as that's indexed on the TYPEIDs. Context: emscripten-core#16711
Due to TypeIDs not being stable between modules when the type info is not visible outside the side-module, embind fails to look up the registered type for a TypeID that has been registered from another module. This is for example the case for emscripten::val, which is registered by embind.cpp. If another module uses the templated val(T&& value) constructor, it will result in a call to _emval_take_value where the passed type id is resolved in the context of the calling module. If these two differ, it will result in a binding error thrown: BindingError: emval::as has unknown type N10emscripten3valE As long as we can't guarantee stable type-ids across modules, we work around the issue by looking for a matching existing registered type based on the type name, and if one is found we register the missing type-id with the same information. Fixes: emscripten-core#16711
58a8459
to
e9f86ce
Compare
Building with visibility=hidden is common practice for libraries such as Qt, where we explicitly and selectively want to export symbols for our APIs. The two type universes are fine in general, but becomes a problem when embind treats them a single universe, by registering types with their universe-specific ID in a global multi-universe registry, but only doing so based on how one of the universes look. That comes crashing down when the other universe wants to also use embind, but the global multi-universe registry doesn't know anything its perspective on IDs. Another workaround would be for every side module to do the same builtin registrations that bind.cpp does, to bring its perspective of IDs into the global multi-universe registry, but that doesn't scale very well. It would also currently fail due to embind not allowing types to be registered twice with the same name. And I'm not sure if that would work for custom type registrations/bindings? Alternatively, emscripten could add and maintain export-macros for all its public types, so that the type-info is exported. Not sure if that's been brought up before? In any case, I assume the intention is that both main modules and side modules should be able to use |
Ping 🙂 Should we add export macros for any types registered by embind instead? |
cc @brendandahl |
This sounds to me like the best solution, if it can be made to work. |
try build main module with -s EXPORTED_FUNCTIONS="['__ZTIN10emscripten3valE']" |
Pull request for exporting Emscripten types: #22095 |
Is there any possibility of merging this PR? Or coming up with some other solution that fixes the same problem? |
@brendandahl have you looked into this issue at all? |
Yeah, I think we had decided to go with the solution in #22095. I thought there was also some issue with that PR, but I can't find any objections. |
I can test if #22095 solves my issue. |
When the shared library is built with symbol visibility hidden as the default, the type infos for any type is not available to anything that links to the library, and the library or executable will have its own copy of the type info, which is not merged.
The results in emscripten::internal::TypeID having different TYPEID values depending on which module does the lookup.
This is problematic for embind, which relies on these TYPEIDs to be stable. For example, in the case of emscripten::val, it is registered by embind.cpp. If another module uses the templated val(T&& value) constructor, it will result in a call to _emval_take_value where the passed type id is resolved in the context of the calling
module.
If these two differ, it will result in a binding error thrown:
BindingError: emval::as has unknown type N10emscripten3valE
As long as we can't guarantee stable type-ids across modules, we work around the issue by looking for a matching existing registered type based on the type name, and if one is found we register the missing type-id with the same information.
Fixes: #16711