Dmitry Katkevich Dmitry Katkevich - 1 month ago 10
C++ Question

Lambda's "this" capture returns garbage

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

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.