Ad

Std::make_shared Leads To Undefined Behavior, But New Works

- 1 answer

Consider the following example class:

class Foo {

public:
    void* const arr_;

    Foo() = delete;

    Foo(const size_t size, bool high_precision) :
        arr_(Initialize(size, high_precision)) {};

    template <typename T>
    T* GetDataPointer() {
        return (T* const)arr_;
    }
private:
    static void* Initialize(const size_t size, bool high_prec) {
        if (high_prec) {
            return new double[size];
        }
        else {
            return new float[size];
        }
    }

};

When I created a shared pointer to a Foo object using std::make_shared, I often find the array data I initialize displays undefined behavior/becomes corrupted. For example:

std::shared_ptr<Foo> sp_foo = std::make_shared<Foo>(Foo(3,false));
    auto foo_data = sp_foo->GetDataPointer<float>();
    foo_data[0] = 1.1;
    foo_data[1] = 2.2;
    foo_data[2] = 3.3;
    std::cout << "First Value: " << 
        sp_foo->GetDataPointer<float>()[0]; // Sometimes this is 1.1, sometimes it is meaningless.

However, when I initialize the shared pointer using new, this problem seems to go away.

std::shared_ptr<Foo> sp_foo (new Foo(3,false));
    auto foo_data = sp_foo->GetDataPointer<float>();
    foo_data[0] = 1.1;
    foo_data[1] = 2.2;
    foo_data[2] = 3.3;
    std::cout << "First Value: " << 
        sp_foo->GetDataPointer<float>()[0]; // Correctly prints 1.1

Any thoughts as to what is happening? Small side note: I am constrained to not template the Foo class and to indeed have a void* const to the onboard data array.

Ad

Answer

Your call to make_shared is using a copy constructor for your Foo class, which you haven't defined. Thus, the default (compiler-generated) copy will be used, and the destructor will be called to delete the temporary. As your class doesn't properly implement the Rule of Three, this (potentially) causes undefined behaviour.

The copy constructor is being used because the argument list in your call to std::make_shared is Foo(3, false) – a Foo object, so the call fits only the first overload listed on this cppreference page. (Note that there is nothing resembling a std::make_shared<T>(const T& src) overload.) From that page, we see:

  1. Constructs an object of type T and wraps it in a std::shared_ptr using args as the parameter list for the constructor of T.

So, with your "args" of Foo(3, false), that make_shared actually calls the following constructor for the object to wrap:

Foo(Foo(3, false))

For correct behaviour, just pass the 3 and false as arguments to make_shared:

std::shared_ptr<Foo> sp_foo = std::make_shared<Foo>(3, false);

You can demonstrate the error in your original code by adding a destructor to the Foo class that logs some output, like: ~Foo() { std::cout << "Destroying...\n"; }. You will see that output after execution of the call to make_shared.

You can prevent this accidental error/oversight by deleting the Foo copy constructor: Foo(const Foo& f) = delete;. This will generate a compiler error along the following lines:

error : call to deleted constructor of 'Foo'


However, in your second case, you use a std::shared_ptr constructor (the third form shown on the linked page). This has no problem, because that constructor 'just' wraps the given pointer into the managed object, and no copying or destruction is needed.

Ad
source: stackoverflow.com
Ad