10

Given the following C interface:

IoT_Error_t aws_iot_mqtt_subscribe(AWS_IoT_Client *pClient,
                                   const char *pTopicName,
                                   uint16_t topicNameLen,
                                   QoS qos,
                                   pApplicationHandler_t pApplicationHandler, 
                                   oid *pApplicationHandlerData);

"aws_iot_mqtt_subscribe stores its arguments for latter reference - to call, in response to some event at some later point in time"

Handler:

typedef void (*pApplicationHandler_t)(
    AWS_IoT_Client *pClient,
    char *pTopicName,
    uint16_t topicNameLen,
    IoT_Publish_Message_Params *pParams,
    void *pClientData);

I am trying to wrap this into a C++ class that would have the following interface:

class AWS {
// ...
public:
  void subscribe(const std::string &topic,
                 std::function<void(const std::string&)> callback);
// ...
};

My goal is to make it possible to pass a capturing lambda function to AWS::subscribe. I have been trying with different approaches for a nearly a week now but none of them seemed to work.

Let me know if anything else needed to understand the problem, I'm happy to update the question.

Jarod42
  • 203,559
  • 14
  • 181
  • 302
haxpanel
  • 4,402
  • 4
  • 43
  • 71

2 Answers2

3

Why it does not work just like that

The reason you can not just pass a C++ function into a C API is because the two have potentially different calling conventions. The extern "C" syntax is to tell the C++ compiler to use the C notation for a single function or for the whole code block if used like extern "C" { ... }.

How to make it work

Create a singleton C++ wrapper around the C API responsible for the initialization/finalization of the latter and forwarding calls and callbacks back and forth. Importantly it should try minimising the amount of raw C++ world pointers into the C API to make clean memory management possible.

godbolt // apologies for the clumsy syntax, too much Java recently :-)

extern "C" {
    void c_api_init();
    void c_api_fini();
    void c_api_subscribe(
        char const* topic,
        void(*cb)(void*),
        void* arg);
}

// this is the key of the trick -- a C proxy
extern "C" void callback_fn(void* arg);

using callaback_t = std::function<void(std::string const&)>;

struct ApiWrapper {
    // this should know how to get the singleton instance
    static std::unique_ptr<ApiWrapper> s_singleton;
    static ApiWrapper& instance() { return *s_singleton; }

    // ctor - initializes the C API
    ApiWrapper(...) { c_api_init(); }

    // dtor - shuts down the C API
    ~ApiWrapper() { c_api_fini(); }

    // this is to unwrap and implement the callback
    void callback(void* arg) {
        auto const sub_id = reinterpret_cast<sub_id_t>(arg);
        auto itr = subs_.find(sub_id);
        if (itr != subs_.end()) {
            itr->second(); // call the actual callback
        }
        else {
           std::clog << "callback for non-existing subscription " << sub_id;
        }
    }

    // and this is how to subscribe
    void subscribe(std::string const& topic, callaback_t cb) {
        auto const sub_id = ++last_sub_id_;
        subs_[sub_id] = [cb = std::move(cb), topic] { cb(topic); };
        c_api_subscribe(topic.c_str(), &callback_fn, reinterpret_cast<void*>(sub_id));
    }

private: 
    using sub_id_t = uintptr_t;
    std::map<sub_id_t, std::function<void()>> subs_;
    sub_id_t last_sub_id_ = 0;
};

Create a C-proxy to bridge between the C API and the C++ wrapper

// this is the key of the trick -- a C proxy
extern "C" void callback_fn(void* arg) {
    ApiWrapper::instance().callback(arg);
}
bobah
  • 18,364
  • 2
  • 37
  • 70
  • 3
    A word of explanation as to why one needs `extern "C"` would benefit this answer. – StoryTeller - Unslander Monica Sep 17 '18 at 08:52
  • 1
    Or, at least, a link to https://stackoverflow.com/questions/2594178/why-do-you-need-extern-c-for-c-callbacks-to-c-functions – Sneftel Sep 17 '18 at 08:53
  • @StoryTeller - good idea, I added the explanation to the top of the question – bobah Sep 17 '18 at 09:14
  • @bobah I might be misunderstanding your answer but my goal is to wrap the C function into a C++ class method thus creating a higher level access / C++ interface of the given C function. – haxpanel Sep 17 '18 at 09:16
  • @melpomene - you are right mate, it was full of typos. please try again, it compiles now. – bobah Sep 17 '18 at 09:57
  • @bobah Thanks for your answer, I'm going to accept melpomene's one because that has relatively small boilerplate code. – haxpanel Sep 17 '18 at 14:13
  • @haxpanel - it's of cause up to you, but you will need this boilerplate code one way or another if you don't want to blow up that API with pointers to dead memory one day ;-) – bobah Sep 17 '18 at 14:15
  • @bobah I'm returning a lambda: `return [=] () { delete callback_copy; };` Which supposed to take care of the memory. Or am I wrong? (I know `new` and `delete` should be avoided and it is preferred to use `unique_ptr`, in a next step..) – haxpanel Sep 17 '18 at 14:20
  • If something the subscription callback needs (or the callback itself) dies before the respective subscription gets removed from the C API then next time the C API is using it for a notification -- the application will end up crashing or corrupting memory. That's why I propose a C++ wrapper around the whole C API to tightly coordinate the lifetime of C API and C++ objects it may be referring to. – bobah Sep 17 '18 at 14:30
  • Let us [continue this discussion in chat](https://chat.stackoverflow.com/rooms/180215/discussion-between-bobah-and-haxpanel). – bobah Sep 17 '18 at 15:15
3

The basic approach is to store a copy of callback somewhere, then pass a pointer to it as your pApplicationHandlerData.

Like this:

extern "C"
void application_handler_forwarder(
    AWS_IoT_Client *pClient,
    char *pTopicName,
    uint16_t topicNameLen,
    IoT_Publish_Message_Params *pParams,
    void *pClientData
) {
    auto callback_ptr = static_cast<std::function<void(const std::string&)> *>(pClientData);
    std::string topic(pTopicName, topicNameLen);
    (*callback_ptr)(topic);
}

This is your (C compatible) generic handler function that just forwards to a std::function referenced by pClientData.

You'd register it in subscribe as

void AWS::subscribe(const std::string &topic, std::function<void(const std::string&)> callback) {
    ...
    aws_iot_mqtt_subscribe(pClient, topic.data(), topic.size(), qos,
         application_handler_forwarder, &copy_of_callback);

where copy_of_callback is a std::function<const std::string &)>.

The only tricky part is managing the lifetime of the callback object. You must do it manually in the C++ part because it needs to stay alive for as long as the subscription is valid, because application_handler_forwarder will be called with its address.

You can't just pass a pointer to the parameter (&callback) because the parameter is a local variable that is destroyed when the function returns. I don't know your C library, so I can't tell you when it is safe to delete the copy of the callback.


N.B: Apparently you need extern "C" on the callback even if its name is never seen by C code because it doesn't just affect name mangling, it also ensures the code uses the calling convention expected by C.

melpomene
  • 84,125
  • 8
  • 85
  • 148
  • `aws_iot_mqtt_subscribe` is given by the AWS C SDK I can't modify it. I'd like to create a C++ class that wraps the C function `aws_iot_mqtt_subscribe`. – haxpanel Sep 17 '18 at 09:22
  • Or what do you mean by "register it as"? – haxpanel Sep 17 '18 at 09:28
  • @haxpanel The call to `aws_iot_mqtt_subscribe` would be in your `subscribe` method. Why do you think you need to modify it? By "register it" I mean "tell the C library to use `application_handler_forwarder` as its callback", which you would do by calling `aws_iot_mqtt_subscribe` as shown above. – melpomene Sep 17 '18 at 09:34
  • Returning a new capturing lambda from `subscribe` could prevent `copy_of_callback` from getting destroyed? That lambda could do the job of `unsubscribe`? – haxpanel Sep 17 '18 at 09:52
  • @haxpanel Well, if you do `auto copy = new std::function(callback); aws_iot_mqtt_subscribe(pClient, topic.data(), topic.size(), qos, application_handler_forwarder, copy);`, then it won't be destroyed automatically. That's easy. But now you have a memory leak. The tricky part is knowing when you can delete the copy. I'm not sure what returning a lambda would buy you. – melpomene Sep 17 '18 at 09:55
  • @haxpanel Oh, if there's an explicit `unsubscribe` operation, that would be the right place to clean up. Sure, you could return a lambda, or a sentinel object that represents a registered callback (then you'd do the unsubscribe / cleanup in its destructor). – melpomene Sep 17 '18 at 09:57
  • I really like your solution looking simple and doesn't require lots of boilerplate code, but it doesn't seem to work. I get exception when it reaches the `application_handler_forwarder ` line `(*callback_ptr)(topic);`. The `copy_of_callback` was created with `new` (the way you mentioned) and passed to `aws_iot_mqtt_subscribe`. Did I do something wrong? – haxpanel Sep 17 '18 at 13:27
  • IT IS WORKING NOW!! I had to remove the `&` reference from `aws_iot_mqtt_subscribe(pClient, topic.data(), topic.size(), qos, application_handler_forwarder, &copy_of_callback);` – haxpanel Sep 17 '18 at 14:09