Oh I am mostly arguing against your claim that UniqueModulePtr is unsafe. I havent looked into this bug properly... > Returning UniqueModulePtr from your factory ABI within a plugin theoretically > sounds like an improvement but requires a few pieces to be moved around and > I'm not yet convinced it's sufficiently safe. No as stated above this is the way it is meant to be used. And you might have noticed that already use it on the server side for that. And a closer look will show you that bug: #1528135 only exists because UniqueModulePtr allowed us to remove global variables that kept plugins alive. > Forgive me for repeating but I want to restate the problem more concisely for > other readers' and my own benefit: > > Let's imagine you have a plugin named mesa.so which implements interface > Platform with its own MesaPlatform class. Your application knows nothing about > "mesa" and contains no symbols with "mesa" in their name. Everything named > "mesa" lives in mesa.so. > Now you load your plugin "mesa.so" and it returns a > UniqueModulePtr. Because the only part of your project that > knows the word "mesa" is mesa.so, the UniqueModulePtr is fully > and only instantiated inside mesa.so. > Now you unload your plugin and three things get torn down, in this order: > 1. ~MesaPlatform inside mesa.so > 2. Unload/unmap mesa.so from memory. > 3. ~UniqueModulePtr which was inside mesa.so, but is no > longer loaded so you crash. The starting idea "Because the only part of your project that knows the word "mesa" is mesa.so, the UniqueModulePtr is fully and only instantiated inside mesa.so." is already wrong. Instead it should read. "A part of mirclient needs to create the ClientPlatfrom to be used, So it loads the right plugin and resolved create_client_platform and stores the returned UniqueModulePtr somewhere, then forgets about the mir::SharedLibrary it needed for that". Then exactly 3 is not happening; the destructor call to the mir::client::ClientPlatform is encoded where the destructor call of the unique_ptr is encoded. The Deleter of that UniqueModulePtr will ensure that the SharedLibrary outlives the ClientPlatform object. Even when someone stores another UniqueModulePtr inside an object 'owned' by mesa.so (==ClientPlatform) wouldnt matter since someone still owns mesa.so(==ClientPlatform) so keeps it alive even when the object created and stored inside for whatever reason goes away. The important part here is that the plugin lifetime is now tied to the object lifetime created form it. If for some reason the ClientPlatform creates objects for others to own that may outlive the ClientPlatform instance those need to be UniqueModulePtr too. See mir::graphics::Platform. Which is kind of a hint that graphics::Platform might be two or three separate things actually. > > Yes, in theory you could avoid the crash, but only by either: > (a) Leaking mesa.so by any means, accidental or intentional; or > (b) Ensuring UniqueModulePtr is only used on the interface: > UniqueModulePtr. > > I think (b) won't work right now without more fixes because UniqueModulePtr > relies on being given the implementation class in order to figure out which > shared library to keep resident. And (a) I know we have done both accidentally > and intentionally, so did not notice the problem for a while. b) even works when you use UniqueModulePtr inside, as long as the lifetime of those instances is tied to objects created at the interface of the plugin. This is not really different than other lifetime semantics. And of course just like with the normal unique_ptr you do not have to use the derived class name in the interface and use the base class instead.