3

Could you please help to review the below code? is there any issue if a local std::function is out of its "life"? Thanks in advance.

class Test {
    public:
    void commit(std::function<void()> func)
    {
        // return immediately
        dispatch_async(conqueue, ^{
            // in order to call func when it is out of "life"
            sleep(5);
            func(); // is there any issue? the func should be invalid at this moment?
        });
    }

    void test() {
        std::string name = "test";
        std::function<void()> func = [name, this] () {
            printf("%lx %s\n", &name, name.c_str());
            std::cout << "callback"<<std::endl;
        };
        // async
        commit(func);
    }
    //...
};
newacct
  • 119,665
  • 29
  • 163
  • 224
  • Do you have any specific problem with it? Should be fine IMO. – πάντα ῥεῖ Oct 19 '20 at 17:56
  • I think the func should be invalid due to the test is returned? I did not see any problem at runtime coz it's a simple code and the memory is not corrupt even it's invalid? is there any exception in some complicated cases? – android2test Oct 19 '20 at 17:58
  • Duplicate question: https://stackoverflow.com/questions/7941562/what-is-the-lifetime-of-a-c-lambda-expression . What you need to be careful about is the lifetime of any captured values in your lambda, unless those are captured by value (as you did). – πάντα ῥεῖ Oct 19 '20 at 17:59
  • I'm not too sure about `std::function` (or C++ class types in general), but this might apply: https://stackoverflow.com/q/40315949/5754656 – Artyer Oct 19 '20 at 18:02
  • What's that `^{...}` thing? That's not in the standard C++. – HolyBlackCat Oct 19 '20 at 18:03
  • I guess it depends on `dispatch_async` and this `^{}` construct. Is this C++-CLI? Please tag it if so (or wherever these things actually are). – Yksisarvinen Oct 19 '20 at 18:03
  • yes, it's objective-c. – android2test Oct 19 '20 at 18:08
  • Thanks, πάντα ῥεῖ. if the captured values are deep copied, it should be safe, right? it should have a problem if just use the reference? – android2test Oct 19 '20 at 18:26
  • @android2test Yes, exactly. `this` might be a problem though, if the class is destructed before the async function call completes. You'll need to use a `std::mutex` or a similar synchronization mechanism to prevent that. (BTW if you want to ping me, put a `@` as prefix before my nickname, otherwise I'll not be noticed in my inbox) – πάντα ῥεῖ Oct 19 '20 at 19:05
  • Thanks a lot, @πάντα ῥεῖ. – android2test Oct 20 '20 at 13:23

2 Answers2

0

OK, I ran some tests and have changed my view on this. The code is safe because func is captured by value (i.e. the block inherits a copy).

I proved this to myself with the following test program (you will need MacOS to run this):

// clang block_test.mm -lc++

#include <iostream>
#include <unistd.h>
#include <dispatch/dispatch.h>

struct Captured
{
    Captured () {}
    Captured (const Captured &c) { std::cout << "Copy constructor\n"; }
    void Foo () const { std::cout << "Foo\n"; }
};

int main ()
{
    Captured c;

    // return immediately
    dispatch_async (dispatch_get_global_queue (DISPATCH_QUEUE_PRIORITY_DEFAULT, 0),
    ^{
        sleep(5);
        c.Foo ();
     });
    
    sleep (10);
};

Output:

Copy constructor
Copy constructor
Foo

I'm not sure why the copy constructor is called twice though. Ask Apple.

Update: There's a good writeup on working with blocks here. That's what that funny 'hat' syntax is.

Paul Sanders
  • 24,133
  • 4
  • 26
  • 48
  • Thanks, Paul. I am not sure whether the func will be copied to block of dispatch_async or not? – android2test Oct 19 '20 at 18:24
  • Ah, that's a good point. Neither am I. You could test that with a simple class who's copy constructor outputs something to `cout`. – Paul Sanders Oct 19 '20 at 18:35
  • @PaulSanders Note that OP also captured `this` in their example. This needs to be handled with care if the lambda is executed asynchonously (see my [comment](https://stackoverflow.com/questions/64432753/is-there-any-issue-if-local-stdfunction-out-of-its-life#comment113934840_64432753) please). – πάντα ῥεῖ Oct 19 '20 at 19:08
  • @πάνταῥεῖ Yes, but I think he was more concerned about capturing `func` in the block. – Paul Sanders Oct 19 '20 at 19:13
  • @paul That's what's covered in the dupe I proposed. – πάντα ῥεῖ Oct 19 '20 at 19:14
0

It's safe. Apple "Blocks" capture non-__block variables by value, so the block contains a copy of the std::function inside it, so the lifetime of the block's copy of the variable is the same as the block itself.

(P.S. Even if the variable was declared __block, it would still be safe. Blocks capture __block variables kind of "by reference", in that multiple blocks, as well as the original function scope, share the state of the variable. However, the underlying mechanism of __block ensures that the variable is moved to the heap if it is used by a block that outlives the function's scope. So for a __block variable, its lifetime lasts until all blocks that captured it have been destroyed.)

newacct
  • 119,665
  • 29
  • 163
  • 224