Is there an ideomatic way to avoid this use of Naked New?

Question

In my application, I need to set callbacks whose sole responsibility is to update the state associated with a completed event in a separate object so that it can be queried later. However, because of how the API is designed, I can't guarantee that the other object will still be owned by the time the event completes, so I need to store a pointer to that object, and because the callback API is C-based, I end up storing a raw pointer to a smart pointer, and it's the ugliest piece of code I've seen*.

^{^{* Well in the last few hours anyways...}}

So this is what I wrote to accomplish this:

event.setCallback(CL_COMPLETE, [](cl_event event, cl_int, void* ptr) {
    auto ptr_ptr = static_cast*>(ptr);
    if(auto shared_ptr = ptr_ptr->lock()) {
        auto & shared_state = *shared_ptr;
        std::lock_guard lock{ shared_state.mutex };
        shared_state.event_state[event] = true;
    }
    delete ptr_ptr;
}, new std::weak_ptr(future.state));

In particular I object to my own use of new std::weak_ptr(future.state), which to me seems like some kind of antipattern: using naked new and delete in conjunction with smart pointers.

The problem, however, is that because the callback has to be a function pointer, my lambda expression cannot copy or reference other objects, and the only way to get the shared_state object inside the lambda is to pass its pointer in; and again, because I cannot guarantee that its lifetime hasn't expired, I need to take it in the form of a pointer to a weak_ptr so that it can be manipulated if (and only if) the object still exists.

So ultimately, my question is: is there an ideomatic way to pass shared_state into this callback where

I can check to make sure the object still exists, while
Also eliminating my use of naked new and delete calls?

Admin · Answer

Within the lambda, there's a clear improvement possible by simply using a unique_ptr:

auto callback = [](cl_event event, cl_int, void* ptr) {
    std::unique_ptr> ptr_ptr{ static_cast*>(ptr)};
    if(auto shared_ptr = ptr_ptr->lock()) {
        std::lock_guard lock{ shared_state->mutex };
        shared_state->event_state[event] = true;
    }
}

On the creation side of things, You could arguably use std::unique_ptr<>::release().

auto ptr = std::make_unique>(future.state);

event.setCallback(CL_COMPLETE, callback, ptr.release());

But since you are calling a C function with a capture-less lambda, it's not like there's unexpected exceptions to protect against here, so it's debatable wether or not that's a real improvement over what you are currently doing.

user_2079303 · Answer

You can store a smart pointer to the object inside a map, with a non-owning pointer as the key. The callback can erase the pointer from the map, thereby releasing the ownership. The map has to outlive the callback. You can resort to static storage if you need to.

With unique ownership, you can instead store the object itself in the map directly.

Whether the extra complexity and overhead of the data structure to track the allocations is worth it to get rid of explicit new and delete is debatable in case of unique ownership. With shared ownership, this would probably be a better choice.

Is there an ideomatic way to avoid this use of Naked New?

2 Answers