Как правильно освобождать память?

Question

[calculator212]

Мой код

#include <iostream>
template <class T>
class Array
{
private:
    int m_length;
    T *m_data;

public:
    Array()
    {
        m_length = 0;
        m_data = nullptr;
    }

    Array(int length)
    {
        m_data = new T[length];
        m_length = length;
    }

    ~Array()
    {
        delete[] m_data;
    }

    void Erase()
    {
       delete[] m_data;
       m_data = nullptr;
       m_length = 0;
    }


    T& operator[](int index)
    {

        return m_data[index];
    }

    int getLength(){return m_length;}
};

int main()
{
    Array<Array<int>> ArrayArray(1);
//    ArrayArray[0] = *new Array<int>{10};
    ArrayArray[0] = Array<int>{10};
    return 0;
}

Проблема такая, что если выделяю память так "ArrayArray[0] = *new Array{10};", то всё отрабатывает нормально, если без new, то приложение падает с double free detected in tcache 2. Как сделать так, чтобы это корректно работало при такой инициализации " ArrayArray[0] = Array{10};".

Comments

[Никита]

С ответами выше полностью согласен, но зачем использовать динамические сишные массивы в C++? Давно изобретен очень удобный std::vector, где все это уже учтено.

[calculator212]

Никита, я пытаюсь разобраться с шаблонами, для этого и делаю

Answer 1

[Евгений Шатунов]

Давай рассмотрим, для начала, строку под комментарием.

ArrayArray[0] = *new Array<int>{10};
Что тут происходит.
ArrayArray[0] вернет ссылку на Array<int>.
*new Array<int>{10} выделяет память в куче под Array<int>, вызывает инициализатор Array<int>::Array(int length), после чего делает разыменование получившегося указателя на Array<int>.
После этого для подготовленной ссылки на Array<int> будет выполнен оператор копирования по умолчанию, функциональность которого просто скопирует поля из объекта справа в объект слева от присвоения.
После этого результат new Array<int>{10} становится утекшим, т.к. указатель на него сразу становится потерян и освободить занимаемую им память становится невозможно.
Именно поэтому с этой строчкой у тебя "всё отрабатывает нормально". И нет, всё у тебя не отрабатывает нормально потому что память утекла.

Давай рассмотрим следующую строку.

ArrayArray[0] = Array<int>{10};
Что тут происходит.
ArrayArray[0] вернет ссылку на Array<int>.
Array<int>{10} инициализирует безымянный локальный объект на стеке, используя инициализатор Array<int>::Array(int length).
После этого выполняется уже оператор перемещения по умолчанию, суть которого снова сводится к копированию полей из объекта справа в объект слева. После этого оба объекта ссылаются на одну и ту же выделенную память через поле T *m_data.
Далее безымянный локальный объект уничтожается, освобождая недавно выделенную память. А ArrayArray[0] в этот момент начинает ссылаться на освобожденную память.

Double deletion происходит тогда, когда ArrayArray в конце работы программы пытается удалить уже освобожденную память в ArrayArray[0].

В твоем коде на лицо нарушение инварианта типа.
Что нужно сделать.
Тебе должно уже стать понятно что проблема в твоем коде связана с поведением операторов копирования и перемещения по умолчанию. Но проблема у тебя, на самом деле, значительно шире. Потому что завтра ты ведь точно захочешь еще и инициализацию копией провести или вернуть объект по значению из функции. В этом случае тебя тоже ждут проблемы.
Решением твоих проблем будет соблюдение правила 3/5/0.
Тебе нужно полноценно описать поведение объектов при копировании, перемещении, а так же при инициализации копией и инициализацией через перемещение.

Comments

[calculator212]

Спасибо за помощь, со всеми ссылками я еще не разобрался, для начала сделал вот так. Можете посмотреть, в правильном ли направлении я иду. После вызова деструктора память очищается, но возможно я все равно что-то сделал не совсем правильно.

Array(const Array &copy){
        m_length = copy.m_length;
        if (m_length > 0)
            m_data = new T[m_length];
        else
            m_data = nullptr;
    }


        Array& operator= (const Array &ai) {

        delete[] m_data;
        m_data = new T[ai.getLength()];
        for (auto i = 0; i < ai.getLength();i++){
            m_data[i] = ai[i];
        }
        m_length = ai.getLength();

        return *this;
    }

операторов копирования и перемещения по умолчанию.

Это же нужно реализовывать при помощи конструкторов копирования и перемещения?

[Евгений Шатунов]

calculator212, у тебя довольно хорошо получается решать проблему. Давай я только немного подправлю твои действия.

Для размерных величин int очень плохо подходит. Для этих целей в стандартной библиотеке есть тип size_t[?].
Дополнительно, стандарт C++11 позволяет нам сегодня использовать инициализацию полей по месту их определения. Этими вещами полезно пользоваться.

Определение состояния

template< typename TValue >
class Array final
{
private:
    TValue* m_memory    = nullptr;
    size_t  m_length    = 0;
};

При таком определении полей код инициализации можно изрядно сократить используя default там где это можно.
C++11 вводит термин перемещения ресурса. Поэтому правило теперь называется не "Правило 3", а правило "3/5/0".

Следуя правилу 3/5/0

public:
    inline Array()  = default;
    inline ~Array() { delete[] m_memory; };
    
    explicit inline Array( const size_t length );
    
    inline Array( const Array& other );
    inline Array( Array&& other );
    
    inline Array& operator = ( const Array& other );
    inline Array& operator = ( Array&& other );

Логика копирования должна быть какой? У тебя массив, в нем данные. Массив копируется. Значит данные тоже надо скопировать.
Обрати внимание на то, что конструктор копирования использует синтаксис делегирующего конструктора чтобы передать конструктору преобразования требуемую длину массива.

Конструктор копирования

template< typename TValue >
inline Array<TValue>::Array( const Array<TValue>& other )
    : Array{ other.m_length }
{
    for( size_t index = 0; index < m_length; ++m_length )
    {
        m_memory[ index ] = other.m_memory[ index ];
    }
}

Если конструктор копирования работает в создаваемом объекте, то оператор копирования работает только в уже созданном объекте. Т.е. в момент работы оператора копирования объект же находится в каком-то состоянии. И это состояние нужно правильно завершить, согласно логике работы твоего шаблона.

Оператор копирования

template< typename TValue >
inline Array<TValue>& Array<TValue>::operator=( const Array<TValue>& other )
{
    if( m_length != other.m_length )
    {
        delete[] m_memory;
        m_length = other.m_length;
        m_memory = new TValue[ m_length ];
    }
    
    for( size_t index = 0; index < m_length; ++m_length )
    {
        m_memory[ index ] = other.m_memory[ index ];
    }
    return *this;
}

Термин перемещения означает буквально перенос состояния из одного объекта в другой. И если копирование приводит к повторному выделению памяти в твоем шаблоне, то перемещение уже не должно.
В этом и суть перемещения.

Перемещение

template< typename TValue >
inline Array<TValue>::Array( Array<TValue>&& other )
    : m_memory{ std::exchange( other.m_memory, nullptr ) }
    , m_length{ std::exchange( other.m_length, 0 ) }
{
}

template< typename TValue >
inline Array<TValue>& Array<TValue>::operator=( Array<TValue>&& other )
{
    delete[] m_memory;
    m_memory = std::exchange( other.m_memory, nullptr );
    m_length = std::exchange( other.m_length, 0 );
    return *this;
}

Ну и на последок, еще осталось только конструктор преобразования определить.

Конструктор преобразования

template< typename TValue >
inline Array<TValue>::Array( const size_t length )
    : m_length{ length }
{
    if( m_length == 0 )
    {
        return;
    }
    
    m_memory = new TValue[ m_length ];
}

Все вместе это работает примерно так.
Я не довожу этот код до оптимальности чтобы тебе было легче ориентироваться в нем. Сравнивай свой код с моим и попробуй уловить все то, почему я написал именно так.

Answer 2

[Wataru]

В деструкторе удаляйте, только если память не nullptr или длина не 0.