7
\$\begingroup\$

Background

For a piece of software that I need to get extensible, I wanted to design a simple compile-time "plugin" system in C++.

My use case was that the program (actually a library) can accept several input formats at the same time according to a requested interface version. There is a factory that selects the right input parser (let's call that the plugin), but the idea was to find a mechanism that doesn't require to change a common file registering all the known plugins, but rather that each files can register itself by the simple fact of being compiled-in.

Here, plugins are just cpp files, as in the project it was not an option to compile a dynamic library and load the plugins from them. That's why it's a "compile-time plugin", though the word "plugin" may not make much sense in this case...

I refered to this SO question: Compile-time plugin / Automated Factory Registration with separate modules constraint

The plugin Mechanism

plugins.h

#include <list>
#include <string>
#include <map>

namespace PluginSystem {

    /* Base class for plugins */
    class IPlugin {
    public:
        virtual void DoSomething() = 0;
    };

    /* 
     * Base class for PluginRegistrar
     * See PluginRegistrar below for explanations
     */
    class IPluginRegistrar {
    public:
        virtual IPlugin* GetPlugin() = 0;
    };

    /* 
     * This is the factory, the common interface to "plugins".
     * Plugins registers themselves here and the factory can serve them on
     * demand.
     * It is a Singleton
     */
    class PluginFactory {
    public:
        /* Get Singleton instance */
        static PluginFactory& Instance();
        /* Register a new plugin */
        void Register(IPluginRegistrar * registrar, std::string name);
        /* Get an instance of a plugin based on its name */
        /* throws out_of_range if plugin not found */
        IPlugin* GetPlugin(std::string name);

    private:
        /* Holds pointers to plugin registrars */
        std::map<std::string, IPluginRegistrar*> registry_;
        /* Make constructors private and forbid cloning */
        PluginFactory(): registry_() {};
        PluginFactory(PluginFactory const&) {};
        void operator=(PluginFactory const&) {};
    };

    /* 
     * Helper class that registers a plugin upon construction.
     * Actually, the registrar registers itself, and the proxied plugin is only
     * created on-demand. This mechanism can be shortened by directly 
     * registering and instance of the plugin, but the assumption here is that
     * instanciating the plugin can be heavy and not necessary.
     */
    template<class TPlugin>
    class PluginRegistrar: public IPluginRegistrar {
    public:
        PluginRegistrar(std::string classname);
        IPlugin* GetPlugin();
    private:
        /* That is not really used there, but could be useful */
        std::string classname_;
    };

    /* template functions in header */

    template<class TPlugin>
    PluginRegistrar<TPlugin>::PluginRegistrar(std::string classname): classname_(classname) {
        PluginFactory &factory = PluginFactory::Instance();
        factory.Register(this, classname);
    }

    template<class TPlugin>
    IPlugin*
    PluginRegistrar<TPlugin>::GetPlugin() {
        return new TPlugin();
    }
}

/*
 * Here is the trick: upon creation of the global variable, the class created
 * out of the template will get instanciated once, and will register itself.
 * The template contains the information to create a plugin instance.
 * An unnamed namespace is used to enclose this later unused variable in the
 * compilation unit.
 */
#define REGISTER_PLUGIN(CLASSNAME) \
    namespace { \
        static PluginSystem::PluginRegistrar<CLASSNAME> \
        CLASSNAME##_registrar( #CLASSNAME ); \
    };

plugins.cpp

#include "plugins.h"

namespace PluginSystem {

PluginFactory&
PluginFactory::Instance() {
    static PluginFactory instance;
    return instance;
}

void
PluginFactory::Register(IPluginRegistrar * registrar, std::string name) {
    registry_[name] = registrar;
}

IPlugin* 
PluginFactory::GetPlugin(std::string name) {
    IPluginRegistrar* registrar;
    registrar = registry_[name]; /* throws out_of_range if plugin unknown */
    return registrar->GetPlugin();
}

}

How to use

plugin1.cpp

/* Could be also split in .h/.cpp, here kept in one file for simplicity */
#include <iostream>
#include "plugins.h"

class Plugin1: public PluginSystem::IPlugin {
    void DoSomething() {
        std::cout << "Plugin1" << std::endl;
    }
};

REGISTER_PLUGIN(Plugin1)

main.cpp

#include <iostream>
#include "plugins.h"

int main()
{
    PluginSystem::PluginFactory &factory = PluginSystem::PluginFactory::Instance();

    PluginSystem::IPlugin* plugin;

    plugin = factory.GetPlugin("Plugin1"); /* should try-catch that */

    plugin->DoSomething();

    return 0;
}

Questions

Actually, it's been a (very) long time I did not code C++, so any advice, suggestion, correction is very welcomed.

I had to conform MSVC 2008 (eek...), which maybe explains some design choices, but I'm also interested in how to code better with newer standards and better supporting compilers.

I hope to post back in SO the solution for others to find, as the single answer is hard to decrypt for a non-guru. But not before getting your precious inputs, of course!

\$\endgroup\$
2
\$\begingroup\$

Interface

class IPluginRegistrar {
public:
    virtual IPlugin* GetPlugin() = 0;
};

Is GetPlugin() ever going to return a nullptr? I don't think that would be appropriate. So this should return a reference (assuming the IPluginRegistrar retains ownership of the plugin). Alternatively it should return a smart pointer that indicates the type of ownership of the object that is being returned.

Another location where you are using pointers inappropriately:

    void Register(IPluginRegistrar * registrar, std::string name);

Can registrar be nullptr? Who owns the registrar object? You need to think about your ownership semantics of the interface. You are using a very C style of just passing pointers around. This is not how things are done in C++ (or at least modern C++).

Yep this is an old trick.

#define REGISTER_PLUGIN(CLASSNAME) \
namespace { \
    static PluginSystem::PluginRegistrar<CLASSNAME> \
    CLASSNAME##_registrar( #CLASSNAME ); \
};

But I am not sure it really but you anything (apart from an ugly macro).

REGISTER_PLUGIN(MyPlugin);
// or
PluginSystem::PluginRegistrar<MyPlugin>   myPluginRegitra;

I prefer the second one as it is easier to see what is actually happening.

C++ Old School

This was how you removed the copy constructor and copy assignment operator in C++03 (but you did it wrong). You are not supposed to have {} just put the ;. In your technique you can actually still use them in some contexts that the compiler can't catch. If you don't provide definitions (by using the ;) then it still compiles fine if you don't use these operators but if you accidentally do use them in contexts where they are allowed you get linker errors.

 private:
    PluginFactory(PluginFactory const&) {};
    void operator=(PluginFactory const&) {}; // Note: usually this would return PluginFactory&

Even then. This is very old school. Modern C++ has a better technique.

    PluginFactory(PluginFactory const&)  = delete;
    void operator=(PluginFactory const&) = delete;

Bugs

registrar = registry_[name]; /* throws out_of_range if plugin unknown */

No it does not. If name is not in the map then it creates an entry in the map and the Value part of the map is initialized with zero-initialization (and since it is a pointer it will creeate nulltr). Now your second line will crash (as you are calling a method through a nullptr).

To not update the map. You must call find.

auto find = registry_.find(name);
if (find == registry_.end()) {
   throw std::out_of_range("Bad Name");
}
return find->second->GetPlugin();

Overall

Its pretty good. Your main issue is just deciding who owns object (do the register functions really need to dynamically create objects). Can you have stateless plugins?

Once you have decided on ownership then you have define the semantics you want to use to describe the ownership and thus how to enforce it.

Note: Ownership semantics is all about who is responsible for deleting the object at the end of its life cycle.

\$\endgroup\$
  • \$\begingroup\$ Thanks a lot for your input. I decided to drop support for VS2008, as it is too old to be useful for an SO question. I imagined the factory could create several instances of the plugin (most extensible concept), so I used unique_ptr to transfer ownership to caller. Concerning the registrar, there is no transfer to the factory, as it is a global variable. So I used raw pointers. Thanks for the bug, actually I actively searched it as I read that it throws an exception but could not find the type. I do not know how I misread that... \$\endgroup\$ – Cilyan Feb 15 '16 at 15:41
  • \$\begingroup\$ That would be awesome if you could have a look at the correction: gist.github.com/Cilyan/a8117124b04b64642646/revisions \$\endgroup\$ – Cilyan Feb 15 '16 at 15:41
  • \$\begingroup\$ @Cilyan: Would love to have a look at the corrections. But I am only going to do it if you post here on CodeReview (so other people will benefit from the processes). \$\endgroup\$ – Martin York Feb 15 '16 at 20:54
  • \$\begingroup\$ @Cilyan: PS. If you are using github; why is there not a "Pull Request" with the changes for review. \$\endgroup\$ – Martin York Feb 15 '16 at 21:02
  • \$\begingroup\$ The gist was only there to make it easier to track and review. The real code I use is unfortunately closed source. I though of a gist because I did not feel like cluttering CR with another question, but I guess that is finally worth it, so let's do that! See you there. :) \$\endgroup\$ – Cilyan Feb 16 '16 at 15:15

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.