c++listpointersmemorycircular-list

When deleting my circular linked list class, the L variable turns into random addresses


I'm making a circular linked list class in C++, everything works as expected, except for when i delete an object made based on my class. In the destructor, i'm deleting every single element plus the L itself at the end, and setting it to nullptr so when the user tries to use the object's functions again, the if (L == nullptr) return; will stop them.

the problem is, the L = nullptr actually works fine but when i use another function like printArray() or any other one like get()(which haven't included in the code) it actually shows that L is equal to 4 very random memory addresses and the last one somehow points back to L itself! (this is actually the situation when i delete the L, but when i set it to nullptr it will be 0x0 meaning it doesn't point to those random addresses).

#include <iostream>
class list
{
    private:
        unsigned int size;
        struct node {
            int item;
            node *next = nullptr;
        };
        node *L;
        bool deleted;
    public:
        list() {
            size = 0;
            L = new node;
            L->next = L;
            deleted = false;
        }
        ~list() {
            if (L != nullptr) {
                popAll();
                std::cout << L->next << std::endl;
                delete L;
                std::cout << L->next << std::endl;
                L = nullptr;
                deleted = true;
            }
        }
        void printArray() {
            if (deleted && L == nullptr) return;
            node *p = L->next;
            std::cout << '[';
            while (p != L) {
                std::cout << p->item;
                if (p->next != L) std::cout << ", ";
                p = p->next;
            }
            std::cout << ']';
        }
        void popAll() {
            if (deleted && L == nullptr) return;
            node *p = L->next;
            while (p != L) {
                node *n = p->next;
                delete p;
                p = n;
            }
            L->next = L;
            size = 0;
        }
};

I also have the deleted boolean but that's just as a fallback for me if this fails again. Because if i use the deleted as a detector for when the object is deleted, it'll work fine but i wanna know why the L is a nullptr after i set it to it in the destructor, but it doesn't change it in the other functions and it's like it's only been deleted and not even set to nullptr.


Solution

  • ...setting it to nullptr so when the user tries to use the object's functions again, the if (L == nullptr) return; will stop them.

    The word "user" is referring here to some code with bugs. It is a bug to access a deleted object (the list instance). We imagine a scenario like this for the "user" code:

    list *mylist = new list;
    // ... some more code
    // ... and when done:
    delete mylist;
    // ...but then they still make an access:
    mylist->printArray();
    

    That last statement is a bug. It leads to undefined behaviour. The user code is bad, it must be fixed. The memory that mylist points to has been freed and might have unrelated content. There is nothing in that memory you can leave behind as a protection, since you told the system that it is free, and that means the system can do with (and write to) it whatever else it can use it for.

    the L = nullptr actually works fine

    That statement in the destructor is unnecessary, and doesn't help. No other code can reliably read that nullptr value after the destructor has completed and the memory has been freed.

    but when i use another function like printArray() or any other one like get() (which haven't included in the code) it actually shows that L is equal to 4 very random memory addresses

    That is not unusual. After the destructor has run, any code that tries to access a method or field of the deleted object represents a bug. That memory that previously held L, is no longer under your code's control, and could be anything. It should not be accessed. Any code that does access it, has a bug.

    I also have the deleted boolean

    That doesn't help. Your destructor sets deleted to true, but that is useless, because the very next moment, when the memory of the list has been freed, that deleted resides in freed memory, so the value at that location could be anything, as it could already be in use for something else. It should not be accessed by your (or user's) code, nor is it reliable.

    if i use the deleted as a detector for when the object is deleted, it'll work fine

    That is just a matter of chance. Accessing the deleted field of a deleted list instance leads to undefined behaviour. Undefined behaviour may mean it does something else, crashes the program, or... does what you expect. It can be determined by factors that are not under the program's control.

    but i wanna know why the L is a nullptr after i set it to it in the destructor, but it doesn't change it in the other functions and it's like it's only been deleted and not even set to nullptr.

    The memory occupied by L has been freed. It can be used for other purposes. It should not be accessed. Any code that does attempt to access it, represents a bug.

    What to do

    Don't worry about bugs in code that is not under your control. If someone writes "user" code that accesses a list instance after they have deleted it, they have introduced a bug in their program. That is their responsibility. You should not try to fix that situation in your class, nor can you. It is the responsibility of the coder to never write code that accesses memory that has been freed.

    Your destructor should not set any instance fields to some other values in the hope that it will serve a purpose after the destructor has finished, as that is useless.

    Instead just do the following in your destructor, and let all coders using your code take their responsibility to write code that does not access freed memory:

            ~list() {
                // It is guaranteed that `L` is not `nullptr`, as there is no code
                //    anymore that sets it to that value.
                popAll(); // Free all data nodes
                delete L; // Delete the "sentinel" node allocated in the constructor
            }
    

    Remove all those initial if statements that try to detect that it is running on a deleted object. That is useless. This should never happen. Your code should never hit a situation where L is nullptr. It must assume that the instance on which it runs, has not been deleted.