compiling is okay but assignment error of shared_ptr occurs

Question

I think it may probably an assignment error of shared_ptr. I wrote some code regarding my error about shared_ptr of vector containing int pointer. The error occurs the 2-th loop of j-loop. Please let me know what's the mistake in the code. And I wonder whether 'delete vec.get()' is correct for free the memory of the vector.

int i,j;
shared_ptr<vector<int*>> vec = NULL;    
for (j = 0; j < 2; j++)
{
    vec = shared_ptr<vector<int*>>(new vector<int*>());

    for (i = 0; i < 5; i++)
    {
        int* ia = new int[10];
        vec->push_back(ia);
    }
    delete vec.get();
}

The sole purpose of `std::shared_ptr` is to stop you from having to use `delete`. You should only `delete` the `int*` inside vector (which are not managed by any smart pointer). — Yksisarvinen, Nov 30 '19 at 14:18
I want to free memory of int[] inside the vector using one line of code 'delete vec.get()'. What is the method to delete shared_ptr? — Jungwoong, Nov 30 '19 at 14:21
@Jungwoong Can you explain how you intend to use this `vec`? As far as I can tell it should just be `vector>`. I don't see any reason to use manual dynamic allocation for the inner array and I also don't see any reason that `vec` itself needs to be wrapped into a (smart) pointer. — walnut, Nov 30 '19 at 14:23
I dont see any problem with 'vector'. It is a kind of style someone do like. What i do is to free the memory of shared ptr in vec at the end of for-loop of j, then newly assign the new memory to the shared ptr. Please show the solution just in my code. — Jungwoong, Dec 01 '19 at 02:04
@Jungwoong The problem is 1. that you are leaking the memory of all the `new` calls and 2. that a `vector` is much more complicated to handle and more easily causes bugs than a `vector>`. One should not use manual memory management without good reason. — walnut, Dec 01 '19 at 02:22
1. That is why i am asking the how to free the momery in the shared ptr. 2. There is a reason to use vector rather. You can consider int* may be the class* or others containing arrays, it depends. — Jungwoong, Dec 01 '19 at 02:39
@Jungwoong The `shared_ptr` has nothing to do with the `new int[10];` calls. If you want to have these memory allocations managed by a smart pointer, the type inside the vector needs to be a smart pointer, not the one outside, e.g `vector>`, but that is very similar to `vector>` with few difference that probably don't matter to you here. But you did not give any requirements why you would need pointers at all so far. If `int*` was some pointer to class I would be saying exactly the same. — walnut, Dec 01 '19 at 13:21

walnut · Answer 1 · 2019-11-30T16:42:50.240

From what you are showing, there is no need for the smart pointer or the raw pointers. The following code has the same effect as what you seem to intend, except that it also properly initializes all the int elements to zero, which your current code does not do:

vector<vector<int>> vec;    
for (int j = 0; j < 2; j++)
{
    vec = {5, vector<int>(10)};
    // Do something with vec
}

The concrete problem with your current code is that you are trying to delete the raw pointer owned by the shared_ptr. The shared_ptr will delete the owned pointer when its own lifetime ends and no other shared_ptr instance referring to the raw pointer exists anymore. That is its purpose.

If you want to delete the int array you allocated for the int* pointers in the vector, then you need to decide which of the pointers at which index you want to delete:

delete[] (*vec)[index];

vec is the shared_ptr, *vec is a reference to the owned vector<int*>, (*vec)[index] is a reference to the int* stored in the vector<int*> at index index. You need to use delete[] instead of delete, because you allocated with the array form of new.

Given the way your code is structured, you would need to call delete[] for each index of the vector once to avoid any memory leak. Since doing that manually before the vector is destroyed violates the RAII principle, one would use std::unique_ptr for the inner int* instead of raw new. That being said, I already mentioned above that I don't see any reason for pointers of any kind at all.

Build Succeeded · Answer 2 · 2019-12-01T05:55:45.107

-1

Below code should work for you:

    for (int j = 0; j < 2; j++)
    {
        auto vec = std::make_shared<vector<int*>>();

        for (int i = 0; i < 5; i++)
        {
            int* ia = new int[10];
            vec->push_back(ia);
        }
        for (int i = 0; i < 5; i++)
        {
            delete[] vec.get()->at(i);
        }
    }

I do not understand requirement of this type of strange code. If you are using smart pointer, Why Share_ptr instead unique_ptr? why to call delete? Why you allocate the memory to int* when can be handled by vector of integer. Think on it. No need to declare and then allocate the memory. Hope this will help you.

edited Dec 01 '19 at 05:55

answered Nov 30 '19 at 16:10

Build Succeeded

1,153
1
10
24

Now you replicated OP's error. You cannot delete the pointer managed by `std::shared_ptr`. You need to delete the pointers allocated by `new int[10]`. I mean that was the whole point of the question. – walnut Nov 30 '19 at 19:15
Looks fine now, what you say? – Build Succeeded Nov 30 '19 at 19:18
`vec.get()` returns the pointer to `vector` that is managed by the `shared_ptr`. You are not allowed to delete it because the `std::shared_ptr` will delete it when its lifetime ends. The pointers that need to be deleted to avoid memory leaks are the `int*` pointers in the vector. – walnut Nov 30 '19 at 19:20
Agree with you 100% – Build Succeeded Nov 30 '19 at 19:22
@walnut, what i want is to free the momery in the vector with int[] without deleting all the pointers contained in the vector. – Jungwoong Dec 01 '19 at 02:17
@Jungwoong You must delete the `int*` in the `vector` individually. The `shared_ptr` does not take care of that. That is why you should use `vector>` instead, which does the deletion for you automatically. – walnut Dec 01 '19 at 02:24
Please just think of vector since lots of different cases exist. – Jungwoong Dec 01 '19 at 03:06
Do you mean that the deletion is done automatically in case if vector> while it not automatically done in vector ? Why is that? What the difference between the two? – Jungwoong Dec 01 '19 at 03:14
Ideally there is no need of allocation of memory on heap area in case of your question. If you have vector instead of array and integer instead of 'integer pointer to some memory location' then everything will be stack area. So, while un-widening of stack then memory is automatically cleared. – Build Succeeded Dec 01 '19 at 05:36
@Jungwoong A `vector` deletes all the memory that it internally allocated when it is destroyed. It does not follow pointers inside the vector's elements to destroy them. That is the difference between owning and not owning memory. Raw pointers should only ever be used if you don't want to own the memory. I suggest you have a look in a good [introductory book for C++](https://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list) which explains these important concepts. If you had `vector` I would be saying the same thing, use `vector` or `vector>`. – walnut Dec 01 '19 at 13:26

compiling is okay but assignment error of shared_ptr occurs

2 Answers2