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

Question

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)

Is `member` a static function taking `std::function`? That's the only possibility I can think of, in which case `o` doesn't even need to stay alive. — Joseph Thomson, Apr 06 '17 at 06:34
`async` is the function which is called with int* as an argument. I will extend my code example. — Patrick B., Apr 06 '17 at 06:38
No. Sorry for the confusion. It could be though. But I didn't want to mix std-stuff. — Patrick B., Apr 06 '17 at 06:40
Let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/140030/discussion-between-joseph-thomson-and-patrick-b). — Joseph Thomson, Apr 06 '17 at 06:42
I think pass by capture list is the most preferred solution. Passing the shared_ptr as argument is also possible, but it is not elegant. — ikh, Apr 06 '17 at 06:43
@JosephThomson Not sure whether you saw the chat message, no it won't be used at all in the `lambda`. — Patrick B., Apr 06 '17 at 06:52
Wouldn't a function signature such as `void async(std::shared_ptr o, std::function);` make a lot more sense here? And then access your connection pointer inside this function through `o->member`? — Elijan9, Apr 06 '17 at 08:27

score 4 · Answer 1 · answered Apr 06 '17 at 13:57

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.

This is the *way* to do it with asio. You can keep sockets/client handlers etc. all alive using this approach and allow them to expire gracefully when there are no longer async events to handle for them. — Nim, Apr 06 '17 at 14:03

score 1 · Answer 2 · answered Apr 06 '17 at 07:01

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).

Agreed, but I can't change the prototype of the async-function I'm using in reality. It is the one of boost::asio (see the link at the end of my question). — Patrick B., Apr 06 '17 at 11:40

score 0 · Answer 3 · answered Apr 06 '17 at 06:34

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).

skypjack · Answer 4 · 2017-04-06T07:05:14.730

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.

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

4 Answers4