2009-08-06 2 views
1

Итак, у меня есть этот 2d динамический массив, содержимое которого я хочу освободить, когда закончите с ним. Однако после деструктора я продолжаю испортить кучу. Кодекс работает отлично (конечно, с утечками памяти), если я прокомментирую деструктор. (Visual Studio 2005)Проблемы с удалением 2D динамического массива в C++ (который, в конечном счете, сохраняется в векторе)

FrameData::FrameData(int width, int height) 
{ 
    width_ = width; 
    height_ = height; 

    linesize[0] = linesize[1] = linesize[2] = linesize[3] = 0; 

    // Initialise the 2d array 
    // Note: uint8_t is used by FFMPEG (typedef unsigned char uint8_t) 
    red = new uint8_t* [height]; 
    green = new uint8_t* [height]; 
    blue = new uint8_t* [height]; 

    for (int i=0; i < height; i++) 
    { 
     red[i] = new uint8_t [width]; 
     green[i] = new uint8_t [width]; 
     blue[i] = new uint8_t [width]; 
    }  
} 

FrameData::~FrameData() 
{ 

    // Delete each column 
    for (int i=0; i < height_; i++) 
    {   
     delete[] ((uint8_t*) red[i]); 
     delete[] ((uint8_t*)green[i]); 
     delete[] ((uint8_t*)blue[i]);  
    } 

    // Final cleanup 
    delete[] red; 
    red = NULL; 

    delete[] green; 
    green = NULL; 

    delete[] blue; 
    blue = NULL;  
} 

Я понятия не имею, что не так с кодом. Только одна вещь, где-то еще, я сделал это в цикле, где произошла катастрофа

FrameData myFrame; 
std::vector<FrameData> frames; 
...snipped... 
frames.push_back(myFrame); 

Это не должно вызывать каких-либо проблем, не так ли? Если я правильно помню, push_back делает копию вместо хранения указателя или ссылки.

PS. Да, я должен использовать векторы. Но мне не разрешают.

Дополнительная информация:

Оператор = и конструктор копирования не определен. Думаю, это причина для этой проблемы.

+0

Почему вы бросаете указатели в деструктор? – UncleZeiv

+1

Как определяются красный, зеленый и синий? – FireAphis

+1

Покажите нам свой конструктор копирования и оператор = для FrameData. –

ответ

5

Ваша проблема в том, как вы догадались, здесь:.

FrameData myFrame; 
std::vector<FrameData> frames; 
...snipped... 
frames.push_back(myFrame); 

Вектор делает копии элементов, которые толкают в Что у вас есть для вашего конструктора копирования и/или operator= для вашего класса? Если у вас нет определенного, версия по умолчанию, которую создает для вас компилятор, просто делает копии членов вашего класса. Это скопирует элементы-указатели red, green и blue в новый экземпляр. Затем старый экземпляр, который вы скопировали, будет уничтожен, когда он выходит из области видимости, в результате чего указатели будут удалены. То, что вы скопировали в вектор, будет иметь недействительные указатели, так как цель указателя будет удалена.

Хорошее эмпирическое правило состоит в том, что если у вас есть какие-либо необработанные указатели, то вам нужно создать конструктор копирования и operator=, который будет правильно обрабатывать эту ситуацию, убедившись, что указатели получают новые значения и не разделяются, или что собственность передается между экземплярами.

Например, класс std::auto_ptr имеет необработанный указатель - семантика конструктора копирования предназначена для передачи права собственности на указатель на цель.

Класс boost::shared_ptr имеет необработанный указатель - семантика состоит в том, чтобы передавать собственность посредством подсчета ссылок. Это хороший способ обработать std::vectors, содержащий указатели на ваш класс - общие указатели будут управлять правом собственности для вас.

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

+0

+1 - Я собирался ответить точно так же. Похоже, что исходный плакат не соответствовал правилу 3 и удаляет указатели, которые были скопированы и удалены. –

+0

Спасибо, добавив, что конструктор копирования действительно решает проблему. Думаю, для меня это тоже хороший урок. – Extrakun

+0

Обратите внимание: если вы создадите конструктор копирования, вам, вероятно, также потребуется написать оператор = для выполнения того же самого. –

0

Вы в курсе, что push_back делает копию, но имеет ли FrameData подходящий экземпляр-конструктор и оператор присваивания?

Кроме того, почему актеры здесь:

delete[] ((uint8_t*) red[i]); 

В C++, если вы должны использовать C-стиль (или переосмысливать) бросок, код почти наверняка неправильно.

1

Если у вас нет конструктора глубоких копий и оператора присваивания для класса FrameData, то мой смысл в том, что компилятор создает конструктор копирования для использования с push_back. Автоматически создаваемые конструкторы копий и операторы присваивания будут делать копию по порядку, что приведет к мелкой копии в этом экземпляре. К сожалению, ваш деструктор не знает об этой копии, поэтому во время копирования существует хорошая вероятность того, что временная копия FrameData будет уничтожена, и все ваши данные будут переданы вместе с ней.

Вызов деструктора снова в этом процессе приведет к тому, что двойной свободный плюс другие распределения могут использовать часть «свободной» памяти. Похоже, это хорошая причина для кучи коррупции.

Лучший способ найти такие проблемы, как правило, использовать такой инструмент, как ValGrind или Purify, чтобы выявить проблему.

1

Это не ответ на ваш вопрос , просто наблюдение.

Поскольку ваши данные кадра может быть большим, чтобы избежать чрезмерного копирования, может быть, это лучше для вас, чтобы использовать

std::vector<FrameData *> frames; 

EDIT: Как уже отмечалось, это также позволит решить проблему сбоев.

Смежные вопросы