Mir

Code review comment for lp:~vanvugt/mir/fix-1527449

Revision history for this message
Daniel van Vugt (vanvugt) wrote :

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.

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<MesaPlatform>. Because the only part of your project that knows the word "mesa" is mesa.so, the UniqueModulePtr<MesaPlatform> 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<MesaPlatform> which was inside mesa.so, but is no longer loaded so you crash.

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<Platform>.

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.

« Back to merge proposal