Capturing raw `this` pointer is almost always a bad idea

2024-11-19 1099 words 6 minutes

Contents

Lifetime problems

As a reminder, I want to start with a classic example of using an invalid this captured in a lambda acting as a delegate .

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38


#include <functional>
#include <iostream>
#include <memory>
#include <string>

class Item {
public:
    explicit Item(std::string name) :
        name{name}
    {}

    std::function<void()> makeHelloDelegate() {
        return [this]{ hello(); };
    }

    void hello() const {
        std::cout << "item: " << name << std::endl;
    }

private:
    std::string name;
};


int main() {
    auto item = std::make_unique<Item>("pencil");

    auto d = item->makeHelloDelegate();

    item.reset();

    // `this` captured in lambda is now invalid

    // undefined behaviour (SIGSEGV most likely)
    d();

    return 0;
}

This is a classic lifetime issue. The object is destroyed but its this pointer is still captured in lambda. An attempt to call the delegate d will be essentially, a use after free leading to undefined behaviour.

Problems like these are usually solved using shared_from_this with an extra care to avoid creating circular references leading to a memory leak.

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31


class Item : 
    public std::enable_shared_from_this<Item> 
{
public:
    static std::shared_ptr<Item> create(std::string name) {
        struct make_shared_enabler : public Item {
            make_shared_enabler(std::string name) :
                Item(std::move(name))
            {}
        };
        return std::make_shared<make_shared_enabler>(name);
    }

    std::function<void()> makeHelloDelegate() {
        std::weak_ptr<Item> selfWeak{shared_from_this()};
        return [selfWeak = weak_from_this()]{ 
            if (auto self = selfWeak.lock()) {
                self->hello(); 
            }
        };
    }

    // ...

protected:
    explicit Item(std::string name) :
        name{name}
    {}

    std::string name;
};

I’ve chosen to keep a std::weak_ptr in the lambda which means that the lifetime won’t be prolonged by the delegate but, at the same time, it’s safe to call the delegate without the need to track object’s lifetime explicitly.

Move semantics

What will happen once move semantics comes into play? In the example below, is it still safe to call the delegate once pencil has been moved to pencil2?

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11


int main() {
    Item pencil{"pencil"};

    auto d1 = pencil.makeHelloDelegate();

    Item pencil2{std::move(pencil)};

    d1();

    return 0;
}

The answer is: it depends. In this case, it is safe to call the delegate as pencil is still around and its this pointer is therefore valid. The results won’t be as expected though as pencil’s data has been moved. The delegate is therefore no longer useful.

Events

Our class might be a receiver of asynchronous events. Item might be interested in being updated by an external event source about its current location. Let’s introduce a theoretical LocationService and modify Item to subscribe to events in its constructor:

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39


class LocationService {
public:
    using Listener = std::function<void(std::string)>;

    void subscribe(Listener listener) {
        listeners.push_back(listener);
    }

    void update(std::string newLocation) {
        for (auto listener : listeners) {
            listener(newLocation);
        }
    }

private:
    std::vector<Listener> listeners;
};

class Item {
public:
    explicit Item(std::string name, LocationService& ls) :
        name{name}
    {
        ls.subscribe([this](auto location) {
            currentLocation = location;
        });
    }

    void hello() const {
        std::cout 
            << "item: " << name 
            << ", is located: " << currentLocation
            << std::endl;
    }

private:
    std::string name;
    std::string currentLocation;
};

These two classes can be used like so:

 1
 2
 3
 4
 5
 6
 7
 8
 9
10


int main() {
    LocationService ls;
    Item pencil{"pencil", ls};

    ls.update("drawer");

    pencil.hello();

    return 0;
}

What happens if we introduce move semantics?

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17


int main() {
    LocationService ls;

    Item pencil{"pencil", ls};
    Item pencil2{std::move(pencil)};

    ls.update("drawer");

    pencil.hello();
    pencil2.hello();

    return 0;
}

// Produces on stdout:
// item: , is located: drawer
// item: pencil, is located:

After move, pencil receives events instead of pencil2 (as the subscription has been made with its this pointer). So, how to fix that?

One potential solution here is an introduction of a layer of indirection. This will take the form of a two step initialisation:

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24


class Item {
public:
    explicit Item(std::string name) :
        name{name}
    {
    }

    void subsribe(LocationService& ls) {
        ls.subscribe([this](auto location) {
            currentLocation = location;
        });
    }

    void hello() const {
        std::cout 
            << "item: " << name 
            << ", is located: " << currentLocation
            << std::endl;
    }

private:
    std::string name;
    std::string currentLocation;
};

This can be used the following way:

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20


int main() {
    LocationService ls;

    Item pencil{"pencil"};
    pencil.subsribe(ls);

    Item pencil2{std::move(pencil)};
    pencil2.subsribe(ls);

    ls.update("drawer");

    pencil.hello();
    pencil2.hello();

    return 0;
}

// Produces on stdout:
// item: , is located: drawer
// item: pencil, is located: drawer

The moved object is now correctly receiving the updates. two step initialisation composes perfectly with shared_from_this. The Item becomes:

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37


class Item : public std::enable_shared_from_this<Item> {
public:
    static std::shared_ptr<Item> create(std::string name, LocationService& ls) {
        struct make_shared_enabler : public Item {
            make_shared_enabler(std::string name) :
                Item(std::move(name))
            {}
        };
        auto item = std::make_shared<make_shared_enabler>(name);
        item->subscribe(ls);
        return item;
    }

    void subscribe(LocationService& ls) {
        ls.subscribe([self = weak_from_this()](auto location) {
            if (auto strong = self.lock()) {
                strong->currentLocation = location;
            }
        });
    }

    void hello() const {
        std::cout 
            << "item: " << name 
            << ", is located: " << currentLocation
            << std::endl;
    }

protected:
    explicit Item(std::string name) :
        name{name}
    {}

private:
    std::string name;
    std::string currentLocation;
};

The above solves any potential lifetime issues and makes sure that any subscriptions are maintained after moves, which can be proven with an explicit move of shared_ptr’s underlying object:

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15


int main() {
    LocationService ls;

    auto pencil = Item::create("pencil", ls);
    auto pencil2 = Item::create("pencil2", ls);
    
    *pencil2 = std::move(*pencil);

    ls.update("drawer");

    pencil->hello();
    pencil2->hello();

    return 0;
}

Conclusion

Capturing this can lead to very tricky problems which are difficult to track. Careful consideration should be taken if there’s no other alternative but relying on raw this pointer capture. It should avoided as much as possible.