Двойной бесплатный деструктор C++
Я пытаюсь реализовать связанный список на С++. Список содержит указатель на тип узла, выделенный в куче.
Код выглядит следующим образом:
#include <memory>
template<typename T>
class node {
public:
node(T v) : value(v) {}
~node() = default;
T value;
node *next;
};
template<typename T, class Allocator = std::allocator<node<T>>>
class linked_list {
private:
node<T>* head;
Allocator alloc;
public:
linked_list() : head(nullptr) {}
~linked_list() {
for (auto start = head; start != nullptr; start = start->next) {
start->~node();
alloc.deallocate(start, 1);
}
}
void push_back(T value) {
node<T> *new_node = alloc.allocate(1);
new (new_node) node<T>(value);
if (head == nullptr) {
head = new_node;
return;
}
head->next = new_node;
head = new_node;
}
};
В main.cpp:
#include "linked_list.hpp"
int main() {
linked_list<int> a;
a.push_back(4);
a.push_back(5);
return 0;
}
Когда я запустил его, я обнаружил двойное освобождение в кеше T2. Что я сделал не так с деструктором?
Даже компилятор находит ошибку: godbolt.org/z/Kevjhenvo
Код нарушает правило 3/5/0: https://stackoverflow.com/questions/4172722/what-is-the-rule-of-three
Не могли бы вы уточнить?
Я пытаюсь реализовать связанный список на С++. Почему вы запутались в использовании placement-new? Почему бы просто не использовать new, а затем перейти к умным указателям?
Снова компилятор объясняет, в чем проблема: после одной итерации, когда объект был удален, start->next (в операторе for) есть доступ для перехода к следующему элементу, что такое UB!
@Kain Это цикл for (auto start = head; start!= nullptr; start = start->next) { start->~node(); alloc.deallocate (начало, 1); } вызывает неопределенное поведение.
@PaulMcKenzie Я читал, что конструкция и уничтожение устарели в С++ 17 здесь: stackoverflow.com/questions/59539057/…
@VladfromMoscow Не могли бы вы объяснить мне, почему это происходит?
@PaulMcKenzie Интересно, была ли цель разрешить замену другого распределителя вместо стандартного. Это был синтаксис, используемый в C++98.
Когда вы вставляете новый узел, его указатель next остается неинициализированным. Как должно работать условие start != nullptr? Кроме того, head->next = new_node; head = new_node; не имеет для меня смысла. Куда указывает head->next после этих заданий?
start = start->next поскольку start уже уничтожено, использование после уничтожения является неопределенным поведением.
Проблема не связана с правилом 3/5/0, так как ничего не копируется и не присваивается. Проблема в использовании после бесплатного!
Правило трех здесь даже отдаленно не применимо.
@Элджей, я понял, спасибо.
Конструкция и уничтожение устарели в С++ 17. Это правда, но ответ, на который вы ссылаетесь, неверен. Правильный способ — пройти через std::allocator_traits, у которого есть статическая функция-член construct(). Смотрите здесь. Это также верно для allocate().
Обратите внимание, что ваши node имеют только указатель на следующий элемент, и когда вы отправляете элемент в список, вы обновляете указатель head на новый элемент, что означает, что у вас нет способа вернуться к первому элементу.
Ответы 1
Это распространенная ошибка новичков. Вы изменили переменную управления циклом.
for (auto start = head; start != nullptr; start = start->next)
{
start->~node();
alloc.deallocate(start, 1);
}
Вы изменили start (удалив память) в теле цикла for, а затем попытались разыменовать указатель, который вы только что удалили, в его выражении продолжения. БУМ! Вам повезло, что библиотека времени выполнения была достаточно умна, чтобы уловить это и выдать вам ошибку «двойного освобождения», а не, скажем, запустить ядерную ракету.
Это одно из мест, где цикл while лучше, чем цикл for.
while (head)
{
auto to_del = head;
head = head->next;
to_del ->~node();
alloc.deallocate(to_del, 1);
}
Я пропустил много комментариев о ваших устаревших методах, потому что они не относятся к вашей проблеме, но если вы действительно хотите заменить другой тип распределителя, вам следует изучить использование allocator_traits для распределения, построения , уничтожение и освобождение ваших элементов.
Есть и другие проблемы, например, push_back совершенно неправильно указывает, куда он вставляет новый узел. Замена head->next = new_node; на new_node->next = head;, по крайней мере, предохранит вашу программу от осиротения всех новых узлов.
Само по себе это не исправит код, потому что реализация push_back() также нарушена.
@ Евг Согласен. Я немного отредактировал, но не хочу влезать в сорняки.
Это start->~node(); очень странно. И зачем использовать новое размещение здесь new (new_node) node<T>(value); ? Я не думаю, что в этой программе требуется размещение new/delete.