CODEWITHSUNDEEP
c++c++11asynchronouslambda
Patrick B.
Patrick B.

Reputation: 12363

How to keep alive a shared_ptr-object of which members are used in an asynchronous function?

Imagine the following code:

void async(connection *, std::function<void(void)>);

void work()
{
  auto o = std::make_shared<O>();
  async(&o->member, [] { do_something_else(); } );
}

async will, for example, start a thread using member of o which was passed as a pointer. But written like this when o is going out of scope right after async() has been called and it will be deleted and so will member.

How to solve this correctly and nicely(!) ?

Apparently one solution is to pass o to the capture list. Captures are guaranteed to not be optimized out even if not used. 

  async(&o->member, [o] { do_something_else(); } );

However, recent compilers (clang-5.0) include the -Wunused-lambda-capture in the -Wextra collection. And this case produces the unused-lambda-capture warning.

I added (void) o; inside the lamdba which silences this warning.

  async(&o->member, [o] { 
      (void) o; 
      do_something_else(); 
    });

Is there are more elegant way to solve this problem of scope?

(The origin of this problem is derived from using write_async of boost::asio)

Upvotes: 2

Views: 2271

Answers (4)

Joseph Thomson
Joseph Thomson

Reputation: 10403

Boost.Asio seems to suggest using enable_shared_from_this to keep whatever owns the "connection" alive while there are operations pending that use it. For example:

class task : std::enable_shared_from_this<task> {
public:
  static std::shared_ptr<task> make() {
    return std::shared_ptr<task>(new task());
  }

  void schedule() {
    async(&conn, [t = shared_from_this()]() { t->run(); });
  }

private:
  task() = default;

  void run() {
    // whatever
  }

  connection conn;
};

Then to use task:

auto t = task::make();
t->schedule();

This seems like a good idea, as it encapsulates all the logic for scheduling and executing a task within the task itself.

Upvotes: 4

skypjack
skypjack

Reputation: 50550

A solution I've used in a project of mine is to derive the class from enable_shared_from_this and let it leak during the asynchronous call through a data member that stores a copy of the shared pointer.
See Resource class for further details and in particular member methods leak and reset.
Once cleaned up it looks like the following minimal example:

#include<memory>

struct S: std::enable_shared_from_this<S> {
    void leak() {
        ref = this->shared_from_this();
    }

    void reset() {
        ref.reset();
    }

private:
    std::shared_ptr<S> ref;
};

int main() {
    auto ptr = std::make_shared<S>();
    ptr->leak();
    // do whatever you want and notify who
    // is in charge to reset ptr through
    // ptr->reset();
}

The main risk is that if you never reset the internal pointer you'll have an actual leak. In that case it was easy to deal with it, for the underlying library requires a resource to be explicitly closed before to discard it and I reset the pointer when it's closed. Until then, living resources can be retrieved through a proper function (walk member function of Loop class, still a mapping to something offered by the underlying library) and one can still close them at any time, therefore leaks are completely avoided.
In your case you must find your way to avoid the problem somehow and that could be a problem, but it mostly depends on the actual code and I cannot say.
A possible drawback is that in this case you are forced to create your objects on the dynamic storage through a shared pointer, otherwise the whole thing would break out and don't work.

Upvotes: 0

Joseph Thomson
Joseph Thomson

Reputation: 10403

I suggest that your async function is not optimally designed. If async invokes the function at some arbitrary point in the future, and it requires that the connection be alive at that time, then I see two possibilities. You could make whatever owns the logic that underlies async also own the connection. For example:

class task_manager {
  void async(connection*, std::function<void ()> f);
  connection* get_connection(size_t index);
};

This way, the connection will always be alive when async is called. Alternatively, you could have async take a unique_ptr<connection> or shared_ptr<connection>:

void async(std::shared_ptr<connection>, std::function<void ()> f);

This is better than capturing the owner of connection in the closure, which may have unforeseen side-effects (including that async may expect the connection to stay alive after the function object has been invoked and destroyed).

Upvotes: 1

Nick
Nick

Reputation: 6846

Not a great answer, but...

It doesn't seem like there's necessarily a "better"/"cleaner" solution, although I'd suggest a more "self descriptive" solution might be to create a functor for the thread operation which explicitly binds the member function and the shared_ptr instance inside it. Using a dummy lambda capture doesn't necessarily capture the intent, and someone might come along later and "optimize" it to a bad end. Admittedly, though, the syntax for binding a functor with a shared_ptr is somewhat more complex.

My 2c, anyway (and I've done similar to my suggestion, for reference).

Upvotes: 0

Related Questions