Context

I am trying to forward ownership of a std::unique_ptr through a lambda. But because std::functions must always be copyable, capturing a std::unique_ptr is not possible. I am trying to use a workaround described here: https://stackoverflow.com/a/29410239/6938024

Example

Here is my example which uses the described workaround

struct DataStruct
{
  /*...*/
};

class ISomeInterface
{
  /*...*/
};

class SomeOtherClass
{
public:
  void GetSomethingAsync(std::function<void(DataStruct)> resultHandler);
};

class MyClass
{
  void Foo(
    std::unique_ptr<ISomeInterface> ptr,
    std::function<void(std::unique_ptr<ISomeInterface>, DataStruct)> callback)
  {
    // I just want to "pass through" ownership of ptr
    std::shared_ptr sharedPtr = 
      std::make_shared<std::unique_ptr<ISomeInterface>>(std::move(ptr));

    mSomeOtherClass.GetSomethingAsync(
      [cb = std::move(callback), sharedPtr](DataStruct data)
      {
        auto unique = std::move(*sharedPtr.get());
        cb(std::move(*sharedPtr), data);
      });
  }

  SomeOtherClass mSomeOtherClass;
};

Question

Does moving away the std::unique_ptr, which is managed by the std::shared_ptr cause any risk? My assumption would be, that the std::shared_ptr should try to delete the std::unique_ptr object, which is now dangling.

Is this code "safe"? Could the std::shared_ptr try to delete a dangling pointer, or can it know, that its managed object was moved away?

I would appreciate to understand the behaviour in this case more clearly. Someone please explain the specifics of what happens with the smart pointers here.

CodePudding user response：

Yes, it is safe. shared_ptr will eventually delete moved-away unique_ptr, but it is safe. Moved-away unique_ptr contains nullptr rather than a dangling pointer - this is what makes unique_ptr work in the first place.

CodePudding user response：

There are no dangling pointers - "moving" an object can't invalidate any pointers to it since it doesn't physically move - but auto unique = std::move(*sharedPtr.get()), transfers ownership of the uniquely-owned object into unique (it does the same as std:unique_ptr<ISomeInterface> unique = std::move(*sharedPtr)).
After that, *sharedPtr owns nothing and the callback receives an "empty" unique_ptr.
What is the point of that ownership transfer? It looks completely unnecessary.

CodePudding user response：

It feels to me me like you are trying to fit a solution to a problem. While your code looks fine (excepting threading issues, etc). The shared pointer will count how many references remain pointing to it, and will only delete the unique pointer once nothing is left that references it. However... it also looks over complicated. Remember that a std::function object can wrap any functor, not just a lambda. A lambda-function is just a light-weight way of expressing an object that is mostly a function. The shared_ptr is a work around for the short-comings of lambdas, so why use one?
The example struct PrintNum here is an example of a non-lambda object used in a std::function object. As long as your functor object supports the right move semantics, and a constructor that "moves" the pointer into the object, then no shared_ptr should be required.

IMHO.

CodePudding user response：

As long as GetSomethingAsync calls it's parameter at most once, you can be fine. If it calls it multiple times, you will have a null pointer after the first invocation.

The way you have written it, you are throwing away your ISomeInterface and never passing it to callback. You also don't need to .get() to dereference.

I wouldn't have a std::shared_ptr<std::unique_ptr<ISomeInterface>>, instead just std::shared_ptr<ISomeInterface>.

class MyClass
{
  void Foo(
    std::unique_ptr<ISomeInterface> ptr,
    std::function<void(std::unique_ptr<ISomeInterface>, DataStruct)> callback)
  {
    mSomeOtherClass.GetSomethingAsync(
      [cb = std::move(callback), shared = std::shared_ptr<ISomeInterface>(ptr.release())](DataStruct data)
      {
        cb(shared.release(), data);
      });
  }

  SomeOtherClass mSomeOtherClass;
};