Lambda's "this" capture returns garbage

Question

I'm implementing my own class which provides lazy initialization of its member. And I've encountered a strange behavior of capturing this in a lambda.

Here is an example which reproduces this error.

//Baz.h
#include <memory>
#include <functional>
#include "Lazy.hpp"

struct Foo
{
    std::string str;

    Foo() = default;
    Foo(std::string str) : str(str) {}
    Foo(Foo&& that) : str(that.str) {  }
};

class Baz
{
    std::string str;

    Lazy<std::unique_ptr<Foo>> foo;

public:
    Baz() = default;
    Baz(const std::string& str) : str(str)
    {
        //lazy 'this->foo' initialization. 
        //Is capturing of 'this' valid inside ctors???.
        this->foo = { [this] { return buildFoo(); } };
    }
    Baz(Baz&& that) : foo(std::move(that.foo)), str(that.str) { }

    std::string getStr() const
    {
        return this->foo.get()->str;
    }

private:
    std::unique_ptr<Foo> buildFoo()
    {
        //looks like 'this' points to nothing here.
        return std::make_unique<Foo>(str); //got error on this line
    }
};

int _tmain(int argc, _TCHAR* argv[])
{
    ///Variant 1 (lazy Foo inside regular Baz):
    Baz baz1("123");
    auto str1 = baz1.getStr();

    ///Variant 2 (lazy Foo inside lazy Baz):
    Lazy<Baz> lazy_baz = { [](){ return Baz("123"); } };
    auto& baz2 = lazy_baz.get(); //get() method returns 'inst' member (and initialize it if it's not initialized) see below
    auto str2 = baz2.getStr();

    return 0;
}

Variant 1 works well.

Variant 2 crashes with this error:

Unhandled exception at 0x642DF4CB (msvcr120.dll) in lambda_this_capture_test.exe: 0xC0000005: Access violation reading location 0x00E0FFFC.

I'm using vc++120 compiler (from VS2013).

Here is my simplified Lazy class:

#pragma once
#include <memory>
#include <atomic>
#include <mutex>
#include <functional>
#include <limits>

template<
    class T,
        typename = std::enable_if_t<
        std::is_move_constructible<T>::value &&
        std::is_default_constructible<T>::value
        >
>
class Lazy
{
    mutable std::unique_ptr<T> inst;
    std::function<T(void)> func;
    mutable std::atomic_bool initialized;
    mutable std::unique_ptr<std::mutex> mutex;

public:
    Lazy()
        : mutex(std::make_unique<std::mutex>())
        , func([]{ return T(); })
    {
        this->initialized.store(false);
    }

    Lazy(std::function<T(void)> func)
        : func(std::move(func))
        , mutex(std::make_unique<std::mutex>())
    {
        this->initialized.store(false);
    }
//... <move ctor + move operator>
    T& get() const
    {
        if (!initialized.load())
        {
            std::lock_guard<std::mutex> lock(*mutex);

            if (!initialized.load())
            {
                inst = std::make_unique<T>(func());
                initialized.store(true);
            }
        }

        return *inst;
    }
};

So my question is: why does this example crashe? Is it valid to capture this inside constructors?

Answer 1

In general, it is valid to capture this inside a constructor. But when doing so, you have to make sure that the lambda does not outlive the object whose this it captured. Otherwise, that captured this becomes a dangling pointer.

Which is precisely what happens in your case. The Baz whose this is captured is the temporary constructed inside the main-scoped lambda (the one created by return Baz("123"). Then, when a Baz is created inside Lazy<Baz>, the std::function is moved from that temporary Baz into the Baz pointed to by Lazy<Baz>::inst, but the captured this inside that moved lambda still points to the original, temporary Baz object. That object then goes out of scope and wham, you have a dangling pointer.

A comment by Donghui Zhang below (using enable_shared_from_this and capturing a shared_ptr in addition to this) provides for a potential solution to your problem. Your Lazy<T> class stores the T instances as owned by a std::unique_ptr<T>. If you change the functor signature to std::function<std::unique_ptr<T>()>, you will get rid of the issue, since the object created by the lazy initialiser will be the same object as the one stored in Lazy, and so the captured this will not expire prematurely.

Answer 2

The problem is that the this captured is a particular object. You copy the lambda without changing the this that is captured. The this then dangles, and your code breaks.

You could use smart pointers to manage this; but you probably instead want to rebase it.

I would modify Lazy. Lazy requires a source as well as a T.

I'd make give it a signature.

template<
  class Sig, class=void
>
class Lazy;

template<
  class T,
  class...Sources
>
class Lazy<
  T(Sources...),
  std::enable_if_t<
    std::is_move_constructible<T>::value &&
    std::is_default_constructible<T>::value
  >
>
{
  std::function<T(Sources...)> func;
  // ...
  Lazy(std::function<T(Sources...)> func)
  // ...
  T& get(Sources...srcs) const {
  // ...
            inst = std::make_unique<T>(func(std::forward<Sources>(srcs)...));
  // ...

Now Baz has a

Lazy<std::unique_ptr<Foo>(Baz const*)> foo;

with tweaks to ctor and getStr:

Baz(const std::string& str) : str(str)
{
    this->foo = { [](Baz const* baz) { return baz->buildFoo(); } };
}

std::string getStr() const
{
    return this->foo.get(this)->str;
}

and in main we state our Baz comes from no source data:

Lazy<Baz()> lazy_baz = { []{ return Baz("123"); } };