Почему деструктор висит

Question

Ниже код работал нормально.Тем не менее, когда я включаю p=&b в GetValue, код завершился ошибкой «Ошибка отладки».Почему?

class A{
int *p; 
public: 
A(){p=nullptr;}
~A(){if(p!=nullptr)delete p;}
void GetValue(int b);
};

void A::GetValue(int b){
*p=b;
//p=&b;  this will cause destructor to hang, why?
}

int main(void){
A B;
B.GetValue(5);
}

Answer 1

Короче говоря, ваш код имеет неопределенное поведение , поэтому все может произойти. Неопределенное поведение начинается здесь:

void A::GetValue(int b){
  *p=b;
  //p=&b;  this will cause destructor to hang, why?
}

p равно nullptr при вызове функции, поэтому *p - неопределенное поведение. Остальная часть вашего кода даже не имеет значения. И это конец истории.

Извините за такую ​​грубость, но использование вами указателей настолько совершенно и совершенно неверно, что мы даже не можем точно знать, каково на самом деле предполагаемое поведение кода. Не используйте указатели, если вам это не нужно. Не используйте динамически выделенную память, если вам это не нужно. Если вам нужно, используйте std::vector, std::string, std::unique_ptr или другие стандартные классы, которые скрывают указатели от вас, чтобы вам не приходилось писать свои собственные деструкторы. Избегайте прямого использования new и delete.

Answer 2

Во-первых, важно, чтобы у вас была только delete память, выделенная с помощью new.В настоящее время ваш класс A хранит указатель p, который не выделяется с помощью new, но вы делаете delete p, как если бы это было.Результатом этого является неопределенное поведение, что означает, что ваша программа не гарантированно ведет себя правильно, и следует ожидать очень странных ошибок.

Во-вторых, в функции A::GetValue(int b); параметр b является временная переменная.Когда вызывается GetValue, в стеке вызовов выделяется некоторое пространство для передачи значения b, где оно находится в течение всего времени жизни функции.Но после возврата GetValue b больше не существует .В то время как C ++ позволяет хранить указатели на недействительную память, вы должны быть осторожны, чтобы не использовать такой указатель.

Чтобы заставить ваш класс A работать правильно, потребуется немало доработок, но япопытаться объяснить, как я иду.Хотя в настоящее время, кажется, не имеет смысла хранить указатель int*, как это сделал бы простой член int, я буду продолжать использовать указатель, чтобы помочь вашему пониманию, и пусть управление необработанным указателем будетучебное упражнение.

Большинство проблем проистекает из A::GetValue(int).Здесь вы храните адрес временной переменной в контексте, где ожидается указатель new.Вместо этого вам следует убедиться, что вы правильно распределили память и не сохраняете указатель на переходный параметр b:

A::GetValue(int b){
    if (p == nullptr){
        // if p is null, it needs to be allocated before being written to
        p = new int(b); // initialize the memory at p to have value b
    } else {
        // otherwise, p has already been allocated, and its old value can be overwritten
        *p = b;
    }
}

Еще одна более тонкая проблема возникает, когда вы пишете следующий код:

A a1, a2;
a1.GetValue(13);
a2 = a1;

После этих строк произойдет следующее: p член a1 будет удален дважды, что приведет к еще большему неопределенному поведению.Виновником является автоматически сгенерированный оператор присваивания копии , который просто копирует p из a1 в a2 по значению.Чтобы это исправить, вам нужно написать свой собственный оператор присваивания и конструктор копирования следующим образом.Конструктор копирования немного сложен, потому что нужно обрабатывать много разных случаев.

class A {
    ...
    A(const A& other) : p(nullptr) {
        if (other.p){
            p = new int(*other.p); // allocate new int and initialize with other's value
        }
    }
    A& operator=(const A& other){
        if (p){
            // if this is managing a pointer
            if (other.p){
                // if other is managing a pointer too, just copy value
                *p = *other.p;
            } else {
                // otherwise, since other is null, delete and make this null
                delete p;
                p = nullptr;
            }
        } else {
            // if this is not managing a pointer
            if (other.p){
                // other is managing a pointer, time to allocate
                p = new int(*other.p);
            }
            // nothing needs to be done if both are null
        }
    }

Важность этого объясняется в Правиле Трех .