Reconciling classes, inheritance, and C callbacks

Question

In my C++ project, I've chosen to use a C library. In my zeal to have a well-abstracted and simple design, I've ended up doing a bit of a kludge. Part of my design requirement is that I can easily support multiple APIs and libraries for a given task (due, primarily, to my requirement for cross-platform support). So, I chose to create an abstract base class which would uniformly handle a given selection of libraries.

Consider this simplification of my design:

class BaseClass
{
public:
    BaseClass() {}
    ~BaseClass() {}

    bool init() { return doInit(); }
    bool run() { return doWork(); }
    void shutdown() { destroy(); }
private:
    virtual bool doInit() = 0;
    virtual bool doWork() = 0;
    virtual void destroy() = 0;
};

And a class that inherits from it:

class LibrarySupportClass : public BaseClass
{
public:
    LibrarySupportClass()
        : BaseClass(), state_manager(new SomeOtherClass()) {}

    int callbackA(int a, int b);
private:
    virtual bool doInit();
    virtual bool doWork();
    virtual void destroy();

    SomeOtherClass* state_manager;
};

// LSC.cpp:

bool LibrarySupportClass::doInit()
{
    if (!libraryInit()) return false;

    // the issue is that I can't do this:
    libraryCallbackA(&LibrarySupportClass::callbackA);

    return true;
}
// ... and so on

The problem I've run into is that because this is a C library, I'm required to provide a C-compatible callback of the form int (*)(int, int), but the library doesn't support an extra userdata pointer for these callbacks. I would prefer doing all of these callbacks within the class because the class carries a state object.

What I ended up doing is...

static LibrarySupportClass* _inst_ptr = NULL;
static int callbackADispatch(int a, int b)
{
    _inst_ptr->callbackA(a, b);
}

bool LibrarySupportClass::doInit()
{
    _inst_ptr = this;

    if (!libraryInit()) return false;

    // the issue is that I can't do this:
    libraryCallbackA(&callbackADispatch);

    return true;
}

This will clearly do Bad Things(TM) if LibrarySupportClass is instantiated more than once, so I considered using the singleton design, but for this one reason, I can't justify that choice.

Is there a better way?

Answer 1

You can justify that choice: your justification is that the C library only supports one callback instance.

Singletons scare me: It's not clear how to correctly destroy a singleton, and inheritance just complicates matters. I'll take another look at this approach.

Here's how I'd do it.

LibrarySupportClass.h

class LibrarySupportClass : public BaseClass
{
public:
    LibrarySupportClass();
    ~LibrarySupportClass();
    static int static_callbackA(int a, int b);
    int callbackA(int a, int b);
private:
    //copy and assignment are rivate and not implemented
    LibrarySupportClass(const LibrarySupportClass&);
    LibrarySupportClass& operator=(const LibrarySupportClass&);
private:
    static LibrarySupportClass* singleton_instance;
};

LibrarySupportClass.cpp

LibrarySupportClass* LibrarySupportClass::singleton_instance = 0;


int LibrarySupportClass::static_callbackA(int a, int b)
{
  if (!singleton_instance)
  {
    WHAT? unexpected callback while no instance exists
  }
  else
  {
    return singleton_instance->callback(a, b);
  }
}

LibrarySupportClass::LibrarySupportClass()
{
  if (singleton_instance)
  {
    WHAT? unexpected creation of a second concurrent instance
    throw some kind of exception here
  }
  singleton_instance = this;
}

LibrarySupportClass::~LibrarySupportClass()
{
  singleton_instance = 0;
}

---

My point is that you don't need to give it the external interface of a canonical 'singleton' (which e.g. makes it difficult to destroy).

Instead, the fact that there is only one of it can be a private implementation detail, and enforced by a private implementation detail (e.g. by the throw statement in the constructor) ... assuming that the application code is already such that it will not try to create more than one instance of this class.

Having an API like this (instead of the more canonical 'singleton' API) means that you can for example create an instance of this class on the stack if you want to (provided you don't try to create more than one of it).

Answer 2

The problem I've run into is that because this is a C library, I'm required to provide a C-compatible callback of the form int (*)(int, int), but the library doesn't support an extra userdata pointer for these callbacks

Can you elaborate? Is choosing a callback type based on userdata a problem?

Answer 3

The external constraint of the c library dictates that when your callback is called you don't have the identification of the "owning" instance of the callback. Therefore I think that your approach is correct.

I would suggest to declare the callbackDispatch method a static member of the class, and make the class itself a singleton (there are lots of examples of how to implement a singleton). This will let you implement similar classes for other libraries.

Answer 4

Dani beat me to the answer, but one other idea is that you could have a messaging system where the call back function dispatch the results to all or some of the instances of your class. If there isn't a clean way to figure out which instance is supposed to get the results, then just let the ones that don't need it ignore the results.

Of course this has the problem of performance if you have a lot of instances, and you have to iterate through the entire list.

Answer 5

The problem the way I see it is that because your method is not static, you can very easily end up having an internal state in a function that isn't supposed to have one, which, because there's a single instance on the top of the file, can be carried over between invocations, which is a -really- bad thing (tm). At the very least, as Dani suggested above, whatever methods you're calling from inside your C callback would have to be static so that you guarantee no residual state is left from an invocation of your callback.

The above assumes you have static LibrarySupportClass* _inst_ptr declared at the very top. As an alternative, consider having a factory function which will create working copies of your LibrarySupportClass on demand from a pool. These copies can then return to the pool after you're done with them and be recycled, so that you don't go around creating an instance every time you need that functionality.

This way you can have your objects keep state during a single callback invocation, since there's going to be a clear point where your instance is released and gets a green light to be reused. You will also be in a much better position for a multi-threaded environment, in which case each thread gets its own LibrarySupportClass instance.

Answer 6

Could your callback choose an instance based on a and/or b? If so, then register your library support classes in a global/static map and then have callbackADispatch() look up the correct instance in the map.

Serializing access to the map with a mutex would be a reasonable way to make this thread-safe, but beware: if the library holds any locks when it invokes your callback, then you may have to do something more clever to avoid deadlocks, depending on your lock hierarchy.