-1

I'm currently attempting to build in a basic plugin system into my application. I'd ideally like to know nothing about the plugin's class information, so I'm making use of the base Plugin class when grabbing the appropriate memory management functions, like so:

  void* handle = nullptr;

  if (!(handle = dlopen(path.c_str(), RTLD_LOCAL | RTLD_NOW))) {
    throw std::runtime_error("Failed to load library: " + path);
  }

  using allocClass = Plugin *(*)();
  using deleteClass = void (*)(Plugin *);


  auto allocFunc = reinterpret_cast<allocClass>(
    dlsym(handle, allocClassSymbol.c_str()));
  auto deleteFunc = reinterpret_cast<deleteClass>(
    dlsym(handle, deleteClassSymbol.c_str()));

  if (!allocFunc || !deleteFunc) {
    throw std::runtime_error("Allocator or deleter not found");
  }

  return std::shared_ptr<Plugin>(
    allocFunc(),
    [deleteFunc](Plugin *p){ deleteFunc(p); }
  );

The alloc/delete functions on the plugin side basically just call new or delete, e.g.:

extern "C" {
  TestPlugin *allocator() {
        return new TestPlugin();
    }

    void deleter(TestPlugin *ptr) {
        delete ptr;
    }
}

My question is basically about the safety of this type mismatch - with the plugin using its own type, but the loader declaring it as the base type. From limited testing nothing appears to go wrong, but I'm not sure if under the hood it's deleting the slice of memory for the Plugin part and not the derived part.

Are there better ways to go about this without the application importing each plugin's headers?

lyptt
  • 1,043
  • 7
  • 22
  • 1
    Using both RTLD_NOW and RTLD_LAZY makes 0 sense – SergeyA Jul 01 '19 at 17:27
  • @SergeyA yeah that's come from sample code, I'm still working my way through this as I've never done dynamic library loading before. – lyptt Jul 01 '19 at 17:29
  • You probably want `RTLD_LOCAL | RTLD_NOW`. – Maxim Egorushkin Jul 01 '19 at 17:30
  • Most things here are non-standard, starting from `dlsym`. The standard does not define its behaviour so it is in a very real sense undefined. However nothing prevents one from using proper types. Is it hard to declare `void *allocator()`, or `return static_cast(whatever)`? – n. 1.8e9-where's-my-share m. Jul 01 '19 at 17:31
  • @n.m. in what sense? I tried using void* but C++ doesn't like that, needs a real class for std::shared_ptr's constructor to be happy. The actual method declarations plugin-side are fine, they seem to work. – lyptt Jul 01 '19 at 17:33
  • One example for you with a C++ factory function (no need for `deleteClassSymbol`): https://stackoverflow.com/a/53411076/412080 – Maxim Egorushkin Jul 01 '19 at 17:35
  • It is undefined in the sense that the standard does not explicitly defines its behaviour, by never mentioning it; and also by the virtue of `void*` being not compatible with function types. "Seems to work" is one common manifestation of undefined behaviour. – n. 1.8e9-where's-my-share m. Jul 01 '19 at 17:36
  • Actually.. the above code is perfectly valid.. Why? Because even though it calls delete through the base class pointer.. the function that is correctly deleting the pointer is deleting correctly and deleting on the child anyway.. Even if it were to delete via the base pointer.. so long as the destructor is virtual, it's fine.. But who cares as long as the implementer of the plugin interface does things correctly, it shouldn't matter to you. Your code is correct. – Brandon Jul 01 '19 at 17:36
  • 2
    @n.m. `dlsym` is a POSIX function and POSIX requires a function pointer to be perfectly castable to `void*` and back, see "Application Usage" in https://pubs.opengroup.org/onlinepubs/9699919799/functions/dlsym.html – Maxim Egorushkin Jul 01 '19 at 17:37
  • @Brandon I thought this might be the case, since the application doesn't actually delete the object, the plugin does. Thanks for confirming! If you put this in an answer I'll accept it. – lyptt Jul 01 '19 at 17:44
  • 1
    In the provided code I see no indication that `TestPlugin` is in any way related to the `Plugin`. – SergeyA Jul 01 '19 at 17:45
  • @MaximEgorushkin POSIX does not place any requirements on C++ implementations. – n. 1.8e9-where's-my-share m. Jul 01 '19 at 17:49
  • @SergeyA TestPlugin derives from the Plugin abstract class in its header. – lyptt Jul 01 '19 at 17:52
  • 1
    You should [edit] your question to show the relationship between `Plugin` and `TestPlugin`. – 1201ProgramAlarm Jul 01 '19 at 18:24

1 Answers1

-1

Leaving the validity of reinterpret-casting a void* to a function pointer aside, it is UB to call a function through a pointer of a different type. It doesn't matter if the types are related, like Plugin* (*)() and TestPlugin* (). It is also UB to access an object of one type through a pointer of a different type, even if these types are derived and base class respectively. For example

Derived derived;
Base *base;
base = &derived; // OK, will access base subobject
base = reinterpret_cast<Base*>(&derived); // not OK, will access derived object

Your code is likely to work as expected with many (but probably not all) popular compilers if only single non-virtual inheritance is involved. There's however no need to take such risks. All of this can be easily fixed simply by having the library expose only Plugin* based interfaces.

extern "C" {
    Plugin *allocator() {
        return new TestPlugin();
    }

    void deleter(Plugin *ptr) {
        delete ptr;
    }
}

This implementation requires Plugin to have a virtual destructor. It is probably a good idea anyway. OTOH the deleter function is not really needed, the client can just call delete Plugin. Better yet, return a shared_ptr from allocator, possibly with a custom deleter.

n. 1.8e9-where's-my-share m.
  • 102,958
  • 14
  • 123
  • 225