C++ runtime error with shared_ptr and builder pattern

Question

I was studying c++ language with shared pointer and builder pattern.

I have written following code that is not working but I don't understand why it emits run-time error.

Could you tell me why it is not working well and how can I solve this problem to work well?

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

using namespace std;

class Popup
{
public:
    Popup(int value, string str){
        this->v = value;
        this->str = str;
    }
    virtual void print() = 0;
    int v;
    string str;
};
typedef shared_ptr<Popup> PopupPtr;

class PopupA : public Popup
{
public:
    PopupA(int v, string str) : Popup(v, str) { }
    virtual void print() {
        cout << "PopupA" << endl;
    }
};
typedef shared_ptr<PopupA> PopupAPtr;

class PopupB : public Popup
{
public:
    PopupB(int v, string str) : Popup(v, str) { }
    virtual void print() {
        cout << "PopupB" << endl;
    }
};
typedef shared_ptr<PopupB> PopupBPtr;


class Builder
{
public:
    PopupPtr popupPtr;
    Builder() { };
    shared_ptr<Builder> init(int value, string str) {
        shared_ptr<Builder> builder;

        switch (value)
        {
        case 1:
            popupPtr = PopupAPtr(new PopupA(value, str));
            break;
        case 2:
            popupPtr = PopupBPtr(new PopupB(value, str));
            break;
        default:
            cout << "default error" << endl;
            break;
        }

        if (popupPtr) {
            builder = shared_ptr<Builder>(this);
        } 
        else {
            cout << "popup is null" << endl;
        }

        if (!builder) {
            cout << "builder is null" << endl;
        }
        return builder;
    }

    PopupPtr build()
    {
        if (!popupPtr) {
            cout << "popup is null" << endl;
        }
        return PopupPtr(popupPtr);
    }

};
typedef shared_ptr<Builder> BuilderPtr;

int main()
{
    BuilderPtr builderPtr = BuilderPtr(new Builder());

    PopupPtr popupPtr1 = builderPtr->init(1, "111111111111")->build();
    popupPtr1->print();

    PopupPtr popupPtr2 = builderPtr->init(2, "222222222222")->build();
    popupPtr2->print();
    return 0;
}

Thanks in advance for your answers and sorry for my poor english. If you don't understand my question please make a comment.

Also, please don't use [`using namespace std;`](http://stackoverflow.com/q/1452721/1171191) and [`endl`](http://chris-sharpe.blogspot.com/2016/02/why-you-shouldnt-use-stdendl.html). — BoBTFish, Feb 29 '16 at 15:23
@Likoed You might want to have a read of [this question](http://stackoverflow.com/questions/712279/what-is-the-usefulness-of-enable-shared-from-this) about `std::enable_shared_from_this`. — TartanLlama, Feb 29 '16 at 15:29
This doesn't answer the question, but why don't you just use a static 'factory' method which returns PopupPtr directly and get rid of Builder all together? — stijn, Feb 29 '16 at 15:34
@stijn if I use factory pattern and Popup needs many parameters I should pass parameters in constructor so I used builder pattern. I know static factory pattern solve this problem but it is not what I want to. — Likoed, Feb 29 '16 at 15:42
@TartanLlama I read the question you recommend it solve my problem! I think it is an answer that I want to! If you add your answer I will select — Likoed, Feb 29 '16 at 15:59
Having many parameters is not really solved by builder - many parameters is usually an anti-pattern.. See http://programmers.stackexchange.com/questions/311297/constructor-with-tons-of-parameters-vs-builder-pattern/311322#311322 for instance — stijn, Feb 29 '16 at 19:54
Question tagged C++11, consider `using PopupPtr = std::shared_ptr` instead of `typedef`. — kfsone, Feb 29 '16 at 21:55

score 3 · Accepted Answer · answered Feb 29 '16 at 15:56

Your problem is this line:

builder = shared_ptr<Builder>(this);

This will not create a copy of the std::shared_ptr already tracking this, nor will it affect the reference count of it. This creates an entirely new shared pointer which will track this independently, causing a double-delete when both of the reference counts hit zero.

Fortunately, the standard library provides a solution to this problem in the form of std::shared_from_this.

First you need to enable this for your class:

class Builder : std::enable_shared_from_this<Builder>
{
    //...
};

Then instead of creating a new std::shared_ptr from this, call std::shared_from_this:

builder = std::shared_from_this();

C++ runtime error with shared_ptr and builder pattern

1 Answers1