search

stlab / libraries

ASL libraries will be migrated here in the stlab namespace, new libraries will be created here.
https://stlab.cc
Boost Software License 1.0
660 stars 65 forks source link

Handling more then one priority_task_system instance #519

Open mrbean-bremen opened 1 year ago

mrbean-bremen commented 1 year ago

We use stlab in several compilation units, which results in priority_task_system instanciated more than once. This had partly been resolved by using wrapper executors for default_executor that reside in one compilation unit, but is still a problem if using stlab::await() in another compilation unit, resulting in crashes on shutdown, as pre_exit will be called only for one instance. It is difficult to handle this the same way as default_executor due to the template parameter in await.

Is there a possibililty to ensure that always the same thread pool (aka priority_task_system instance) is used? If not, would it make sense to add the possibility to inject the pts, for example by adding it as an optional parameter to await and a couple of other functions?

Edit: The use of "compilation unit" here is probably misleading. What I actually mean are dynamic libraries, not single cpp files, e.g. stlab::await() is used in different sos. This is is also only a problem under linux, as Windows and macOs use the system thread pool and do not use the singleton.

FelixPetriconi commented 1 year ago

I already talked with him verbally about this problem. I think we should state that using the default_executor from a shared library is problematic and it is even more complicated when the user use our own thread pool implementation. Keep this issue open, I will add something to the docs.

mrbean-bremen commented 1 year ago

Just for the record: the issue has been resolved on our side (with the help of @FelixPetriconi) by copying over the await implementation into that common compilation unit, and let it access the priority_task_system instance via a separate function compiled into that unit.

sean-parent commented 1 year ago

The best answer here is to build your dynamic libraries with default visibility. Another option would be to provide an export list to the linker for all the stlab namespace symbols (--export-dynamic-symbol-list=<file> if using ld or lld or a .def file on Windows). You must ensure that all dynamic libraries are built with the same stlab version and configuration.

I'd consider a PR the used [[gnu::visibility("default")]] in the required places - but if you go this route, please propose an example here so we can see how much it mucks with the code.

mrbean-bremen commented 1 year ago

Thanks - yes, we are building all libraries with the same version and configuration. About the visibility - would that really help in this case? The problem here is that the implementation is all in header files (including the static allocation of the singleton), so there will be a separate instance created for each compilation unit. Would the linker be able to merge these, if using default visibilty? I've never used that, so I may have to read up on that first...

sean-parent commented 1 year ago

About the visibility - would that really help in this case?

Yes, linking with default visibility would fix it. All template instantiations are marked as "inline" to the linker so the linker will use the first instance it sees in all locations. If you are not linking with default visibility you are almost certainly causing (other) ODR violations as well.

mrbean-bremen commented 1 year ago

Thank you, I will look into it!