9

I've been badly bitten by this strange behavior of lambdas under XCode today - after trying to trace several memory leaks in iOS around the code, I've narrowed it down to this (and similar) snippet(s) where I assign the ownership of something to a deferred task using a shared pointer:

void DBStorage::dispose(std::shared_ptr<DataChunk>& dc)
{
    backgroundQueue.queueTask([=]() {
        assert( dc.use_count() == 1 );

        if (dc->isDirty()) {
            //store to disk
        }
    });
}

(Note that the shared pointer's use count is always 1 when the lambda is run)

After execution, this task is null-ified with pendingJob = nullptr; which I expected to call the destructor of all the captured-by-value objects, and consequently DataChunk's destructor. However, it looks like that under XCode/LLVM lc's destructor is never called; calling its dtor explicitly, using mutable, and deleting the std::function with a simple delete didn't work either.

Is this standard behavior? I can of course manually call dc.reset() and it works as expected, but this quite makes the point of using a shared pointer moot.


Solution Apparently, it is a known gcc bug.


Contrib

Stand-alone sample with output from Xcode 5.0.2/clang 3.3

#include <iostream>
#include <memory>

void fnRef(std::shared_ptr<int>& ptr)
{
    auto lambda = [=]() { std::cout << ptr.use_count() << ':' << __PRETTY_FUNCTION__ << '\n'; };
    lambda();
}

void fnVal(std::shared_ptr<int> ptr)
{
    auto lambda = [=]() { std::cout << ptr.use_count() << ':' << __PRETTY_FUNCTION__ << '\n'; };
    lambda();
}

int main()
{
    std::shared_ptr<int> ptr(new int);
    for (int i=0; i<10; ++i)
        fnVal(ptr);
    std::cout << '\n';

    for (int i=0; i<10; ++i)
        fnRef(ptr);

    return 0;
}

LLVM/GCC Output

3:void fnVal(std::shared_ptr<int>)::<anonymous class>::operator()() const
3:void fnVal(std::shared_ptr<int>)::<anonymous class>::operator()() const
3:void fnVal(std::shared_ptr<int>)::<anonymous class>::operator()() const
3:void fnVal(std::shared_ptr<int>)::<anonymous class>::operator()() const
3:void fnVal(std::shared_ptr<int>)::<anonymous class>::operator()() const
3:void fnVal(std::shared_ptr<int>)::<anonymous class>::operator()() const
3:void fnVal(std::shared_ptr<int>)::<anonymous class>::operator()() const
3:void fnVal(std::shared_ptr<int>)::<anonymous class>::operator()() const
3:void fnVal(std::shared_ptr<int>)::<anonymous class>::operator()() const
3:void fnVal(std::shared_ptr<int>)::<anonymous class>::operator()() const

2:void fnRef(std::shared_ptr<int> &)::<anonymous class>::operator()() const
3:void fnRef(std::shared_ptr<int> &)::<anonymous class>::operator()() const
4:void fnRef(std::shared_ptr<int> &)::<anonymous class>::operator()() const
5:void fnRef(std::shared_ptr<int> &)::<anonymous class>::operator()() const
6:void fnRef(std::shared_ptr<int> &)::<anonymous class>::operator()() const
7:void fnRef(std::shared_ptr<int> &)::<anonymous class>::operator()() const
8:void fnRef(std::shared_ptr<int> &)::<anonymous class>::operator()() const
9:void fnRef(std::shared_ptr<int> &)::<anonymous class>::operator()() const
10:void fnRef(std::shared_ptr<int> &)::<anonymous class>::operator()() const
11:void fnRef(std::shared_ptr<int> &)::<anonymous class>::operator()() const

IDEOne.com Output for same code

3:fnVal(std::shared_ptr<int>)::__lambda1
3:fnVal(std::shared_ptr<int>)::__lambda1
3:fnVal(std::shared_ptr<int>)::__lambda1
3:fnVal(std::shared_ptr<int>)::__lambda1
3:fnVal(std::shared_ptr<int>)::__lambda1
3:fnVal(std::shared_ptr<int>)::__lambda1
3:fnVal(std::shared_ptr<int>)::__lambda1
3:fnVal(std::shared_ptr<int>)::__lambda1
3:fnVal(std::shared_ptr<int>)::__lambda1
3:fnVal(std::shared_ptr<int>)::__lambda1
2:fnRef(std::shared_ptr<int>&)::__lambda0
2:fnRef(std::shared_ptr<int>&)::__lambda0
2:fnRef(std::shared_ptr<int>&)::__lambda0
2:fnRef(std::shared_ptr<int>&)::__lambda0
2:fnRef(std::shared_ptr<int>&)::__lambda0
2:fnRef(std::shared_ptr<int>&)::__lambda0
2:fnRef(std::shared_ptr<int>&)::__lambda0
2:fnRef(std::shared_ptr<int>&)::__lambda0
2:fnRef(std::shared_ptr<int>&)::__lambda0
2:fnRef(std::shared_ptr<int>&)::__lambda0

Visual Studio 2013 Output

3:fnVal::<lambda_67137a3f93ee478c018cc7068004c9fd>::operator ()
3:fnVal::<lambda_67137a3f93ee478c018cc7068004c9fd>::operator ()
3:fnVal::<lambda_67137a3f93ee478c018cc7068004c9fd>::operator ()
3:fnVal::<lambda_67137a3f93ee478c018cc7068004c9fd>::operator ()
3:fnVal::<lambda_67137a3f93ee478c018cc7068004c9fd>::operator ()
3:fnVal::<lambda_67137a3f93ee478c018cc7068004c9fd>::operator ()
3:fnVal::<lambda_67137a3f93ee478c018cc7068004c9fd>::operator ()
3:fnVal::<lambda_67137a3f93ee478c018cc7068004c9fd>::operator ()
3:fnVal::<lambda_67137a3f93ee478c018cc7068004c9fd>::operator ()
3:fnVal::<lambda_67137a3f93ee478c018cc7068004c9fd>::operator ()

2:fnRef::<lambda_70f241d4201227663d23c74be170d302>::operator ()
2:fnRef::<lambda_70f241d4201227663d23c74be170d302>::operator ()
2:fnRef::<lambda_70f241d4201227663d23c74be170d302>::operator ()
2:fnRef::<lambda_70f241d4201227663d23c74be170d302>::operator ()
2:fnRef::<lambda_70f241d4201227663d23c74be170d302>::operator ()
2:fnRef::<lambda_70f241d4201227663d23c74be170d302>::operator ()
2:fnRef::<lambda_70f241d4201227663d23c74be170d302>::operator ()
2:fnRef::<lambda_70f241d4201227663d23c74be170d302>::operator ()
2:fnRef::<lambda_70f241d4201227663d23c74be170d302>::operator ()
2:fnRef::<lambda_70f241d4201227663d23c74be170d302>::operator ()
Community
  • 1
  • 1
Tom89
  • 264
  • 4
  • 12
  • 1
    What version of the compiler are you using? There was a bug in GCC that a capture-by-value of a reference was a reference. http://stackoverflow.com/questions/6529177/capturing-reference-variable-by-copy-in-c0x-lambda – Dave S Feb 19 '14 at 18:22
  • `clang --version` says,`Apple LLVM version 5.0 (clang-500.2.75) (based on LLVM 3.3svn)` - that definitely looks possible, however first copying `dc` in the local scope and then capturing the copy didn't work either! – Tom89 Feb 19 '14 at 18:31
  • Tom, you're not seeing things I *just* had the same problem writing up an example code for this, and now I can't reproduce it for the life of me. When I say "just" I mean 5 minutes ago. I changed a `std::cout` location, and now its no longer happening, and changing it *back* doesn't matter. And no, I'm not making this up. I'll see i I can do it again. (and I'm running the same Xcode/clang you are, btw). – WhozCraig Feb 19 '14 at 18:32
  • I noobed out when trying to copy to the local scope... `auto local = lc` will not copy the pointer! The code suggested by @Yakk works :) – Tom89 Feb 19 '14 at 18:40
  • @Tom89 Maybe so, but you may find the output I contributed to your question interesting. I certainly did. – WhozCraig Feb 19 '14 at 18:45
  • Yes, it seems to agree with the idea that references are copied by reference, and that there seems to be a bit of disagreement on what this should do. Visual Studio 2013 agrees with IDEOne and outputs 3 and 2. – Tom89 Feb 19 '14 at 18:58
  • Oh its worse than that. I'm hesitantly confident its a defect. I have one more update (and I'll try and cut down the output size, sorry about that). – WhozCraig Feb 19 '14 at 19:02
  • I've added the VS output, but now it's a tad long :) – Tom89 Feb 19 '14 at 19:04
  • @Tom89 Maybe. I've seen worse. =P – WhozCraig Feb 19 '14 at 19:05

1 Answers1

5

As noted by @DaveS this could be a known gcc bug -- captured references are stored as references.

A good rule of thumb when working with stored lambdas is to avoid =, as stored state should be treated with care.

void DBStorage::dispose(std::shared_ptr<DataChunk>& dc)
{
  std::shared_ptr<DataChunk> data_to_store = dc;
  backgroundQueue.queueTask([data_to_store]() { // maybe add `,this` to the capture list
    assert( data_to_store.use_count() == 1 );
    if (data_to_store->isDirty()) {
      //store to disk
    }
  });
}

or:

void DBStorage::dispose(std::shared_ptr<DataChunk> data_to_store)
{
  backgroundQueue.queueTask([data_to_store]() { // maybe add `,this` to the capture list
    assert( data_to_store.use_count() == 1 );
    if (data_to_store->isDirty()) {
      //store to disk
    }
  });
}

as a second bit of unsolicited advice, std::functions are not lambdas, and calling one theLambda is misleading.

Community
  • 1
  • 1
Yakk - Adam Nevraumont
  • 262,606
  • 27
  • 330
  • 524
  • Thanks, I noobed out when trying this by myself as I wrote `auto data_to_store = dc` getting yet another reference! Also, `theLambda` is actually called `pendingJob` in the actual code, but I'll edit the OP to make it less misleading :) – Tom89 Feb 19 '14 at 18:42
  • And... how so a known GCC bug applies as-is to a completely different compiler? That's quite strange, and looks like disagreement on the spec? – Tom89 Feb 19 '14 at 18:45
  • 1
    @Tom89 Wait, `auto data_to_store = dc;` should be a value -- `auto` deduces like a `T` parameter. /puzzle – Yakk - Adam Nevraumont Feb 19 '14 at 18:59
  • I stand corrected again, `auto data_to_store` does indeed work. Never trust "proof" you remember from multi-hour bug hunts! Probably something else was broken when I tried then, who knows. – Tom89 Feb 19 '14 at 19:11
  • Considering that `[&]` is even worse, one might recommend avoiding implicit captures altogether (which I think is probably your intent). – Casey Feb 19 '14 at 20:28
  • 2
    @Casey `[&]` only when creating a lambda that dies (with all copies) before the end of the current scope. `[vars,explicit]` when creating a persistent lambda. `[=]` when writing throw-away code or in the middle of refactoring. – Yakk - Adam Nevraumont Feb 19 '14 at 20:45
  • "A good rule of thumb when working with stored lambdas is to avoid `=`," That doesn't make sense. With stored lambdas it *only* makes sense to use `=` -- you cannot use `&`. – newacct Feb 19 '14 at 23:59
  • @newacct `[=]` -- avoid. `[list,variables,explicitly]` -- all good. Implicit capture of persistant state is dangerous with non-smart pointers, including `this`, everywhere. – Yakk - Adam Nevraumont Feb 20 '14 at 01:57