Причины утечек памяии?

Question

[Кирилл Жиляев]

Уже 2 недели бьюсь над утечками в памяти. Есть полиморфный класс C (Наследует B а B в свою очередь A). Есть вектор с экземпляроми A в него заносятся как классы B так и классы C. С классами B проблем нет (заносятся и выносятся все ОК). Когда программа работает с классами C то начинается вакханалия. Уходит RAM. Первое время думал: "ну забыл где то что то удалить". Создал map в который записывал вызоыв конструкторов и деструкторов. Все в норме.

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

Valgrid внятного ничего не показал.

spoiler

lina.funroyale.com/ghost.rar - соурс. CGameBase самый верхний класс в иерархии. CGame - второй. CGamePetri - проблемный третий. CpetroAction описан в statspetri.
lina.funroyale.com/leak.xml - лог с valgrind

Comments

[Ariox41]

Явно запретите конструктор копирования и перемещения в A и, если есть, удалите их из потомков. Если программа перестала компилироваться - в этом месте была ошибка из-за срезки. Если копирование в этом месте всё же нужно - создайте виртуальный метод copy в класса A и определите в потомках.

Ну или выложите код.

[Кирилл Жиляев]

Ariox41, Выложил код. Под спойлером в вопроск

Answer 1

[jcmvbkbc]

Есть вектор с экземплярами A в него заносятся как классы B так и классы C.

Подсказывает, что объекты твои обрезаются по классу A, т.е., возможно, происходит совсем не то, что ты ожидал.

Если нужен код могу загрузить в репозиторий.

Загружай и показывай.

Comments

[Кирилл Жиляев]

Выложил код. Под спойлером в вопроск

[jcmvbkbc]

Кирилл Жиляев, std::vector<>::erase нельзя использовать так, как ты делаешь в CStatsPetri :: ~CStatsPetri( ). Можно было написать i = m_Messages.erase( i );. Или вообще не вызывать erase в деструкторе, а просто пройти циклом по элементам удаляя их.
И я вижу, что этот неправильный паттерн повторяется и в других местах, где использован erase.

Answer 2

[Vitaly]

Открыли виртуальные деструкторы? Прекрасно! Пришло время открыть умные указатели. Простое правило для новичков: вообще никаких явных new и delete, всё везде передаётся и возвращается с помощью умных указателей (конст ссылок на них). Потом уже можно почитать/посмотреть Саттера, и начать что-то передавать по ссылкам или голым указателям. Но это уже следующий уровень.

Comments

[Кирилл Жиляев]

Писал бы я проект с 0 то может быть вообще JAVA использовал бы. А так 90% когда была готова, а остальные 10% я заточил в 2 класса. Да по логике если и использовать умный указатель то использрвать везде. А это много правок по проекту

[Vitaly]

Кирилл Жиляев, Если вы сопоставите время, затраченное на поддержку проекта без умных указателей и с, вы удивитесь. Время, которое будет затрачено на правки окупиться с лихвой. Никто вам не мешает использовать умные указатели только в вашем куске, и получить существенно более простой и надёжный код.
Использование полностью ручного управления памятью требует определённой квалификации. Если её нет, то обёртки вроде умных указателей -- идеальный вариант. Но, даже есть квалификация есть, то всё равно их же и используют. В современном C++ не пользуются явными new и delete, кроме некоторых редких случаев.

Answer 3

[Ariox41]

В деструкторе CGamePetri ошибка:

for( vector<CPetriAction *> :: iterator i = m_Messages.begin(); i != m_Messages.end();  )
{
     delete *i;
     m_Messages.erase( i );
}

Когда вызывается erase итератор становится невалидным. На практие скорее всего удаляются элементы 0, 2, 4 ... в первой половине вектора, т.к. когда удаляется нулевой элемент - первый становится нулевым, второй первым, и итератор переходит на новый первый элемент, который раньше был вторым. Решается легко:

for( vector<CPetriAction *> :: iterator i = m_Messages.begin(); i != m_Messages.end();  )
{
     delete *i;
}
//m_Messages.clear(); // - в данном случае смысла нет, т.к. это деструктор

Или еще проще:

for(auto& ptr:  m_Messages){
   delete ptr;
}

Если в m_Messages хранить unique_ptr - тогда и этого не нужно, объекты удалятся автоматически в деструкторе вектора.

Comments

[jcmvbkbc]

Решается легко:

for( vector<CPetriAction *> :: iterator i = m_Messages.begin(); i != m_Messages.end();  )
{
     delete *i;
}

Потерял ++i в цикле.
Ну и так, вообще, иногда полезно заглянуть в другие ответы и комментарии к ним.

[Кирилл Жиляев]

for( vector<CPetriAction *> :: iterator i = m_Messages.begin(); i != m_Messages.end(); i++  )
		delete *i;

Сделал так. Но помойм не помогло

[Кирилл Жиляев]

jcmvbkbc, это явно не забытый CPetriAction.

[jcmvbkbc]

Кирилл Жиляев, я в комментарии к своему ответу написал, что этот паттерн удаления встречается не в одном месте. Будь внимателен, что ли?