Cleaning up a stack of pointers

Question

I'm having a few problem with cleaning a stack of pointers. In the below the line with the delete crashes: "memory fault/segmentation fault".

std::stack<reports*> stack;
while(db.fetch())
{
    reports* report = new report(db);
    QThreadPool::globalInstance()->start(report);
    stack.push(report);
}
while( QThreadPool::globalInstance()->activateThreadCount() != 0 );

while( !stack.empty() )
{
    delete stack.top();
    stack.pop();
}

The context of this code is I think not relevant. Except that: db is passed by reference to report constructor, which immediately copy the necessary current row data as non pointer members. Can somebody give me a hint ?

EDIT:

Self answer:

Ok I was touch by god lights just after writing my question.

by default

QThreadPool::globalInstance()->start(report);

will take ownership of the object. Adding the following line in the loop solves the problem:

report->setAutoDelete(false);

Or symply not cleaning up... myself and let Qt Do it.

Use a [smart pointer](http://en.cppreference.com/w/cpp/memory/unique_ptr)? — Some programmer dude, Aug 08 '13 at 08:40
I might be wrong, but why are you deleting `stack.top()` and then `pop()`? It will be an instacrash I suppose. — Levente Kurusa, Aug 08 '13 at 08:41
Thread concurrency issues perhaps? Use a mutex to lock each contested object before performing actions on them. — , Aug 08 '13 at 08:42
what does the `report` destructor look like? does `reports` base class have a virtual destructor? — AndersK, Aug 08 '13 at 08:43
@Levente Kurusa delete stack top will delete the content pointed by the adress at stack top. Poping will take the address of the top of the stack. — , Aug 08 '13 at 08:49
@claptrap well tought. Though there is no static memebres of pointers in report class. So it is not the problem. I answered my own question below. — , Aug 08 '13 at 08:50
You should use `pool->waitForDone()` method to wait until all tasks are done. — Pavel Strakhov, Aug 08 '13 at 08:55
@PavelStrakhov thanks I did not know this one. I'll change this. Can it make a difference in some case ? — , Aug 08 '13 at 08:57
I think it uses system waiting APIs and produces less CPU load than custom infinite loop. — Pavel Strakhov, Aug 08 '13 at 08:59
Why have my question been voted down ? and my auto answer up ???????? — , Aug 08 '13 at 09:01
You can actually answer the question yourself in an answer section and tick it as the correct answer. That way people can quickly see that you've solved your problem without having to read the whole question. — TheDarkKnight, Aug 09 '13 at 08:40

score 1 · Accepted Answer · answered Aug 08 '13 at 08:45

1

Ok I was touch by god lights just after writing my question.

by default

QThreadPool::globalInstance()->start(report);

will take ownership of the object. Adding the following line in the loop solves the problem:

report->setAutoDelete(false);

Or symply not cleaning up... myself and let Qt Do it.

answered Aug 08 '13 at 08:45

You should add this to your original post. – Lachlan Easton Aug 08 '13 at 08:49

score 0 · Answer 2 · edited May 23 '17 at 10:25

0

You could do two things to avoid the explicit memory magnament, and solve your problems:

Use smart pointers.
Use references. In the case of the stack, one of the requeriments of STL containers is that the elements must be copyable. You can solve this wrapping the reference with std::ref.

In this case I think to use a std::shared_ptr is the best way.

edited May 23 '17 at 10:25

Community

1
1

answered Aug 08 '13 at 08:46

Manu343726

13,969
4
40
75

Well I believe pointers can always be copied ? It is just a memory address ? – Aug 08 '13 at 08:47
@user2346536 Yes, raw-pointers can be copyed. But you have to manage its lifetime explicitly. Read the smart pointers link of the answer. – Manu343726 Aug 08 '13 at 08:50

score -1 · Answer 3 · answered Aug 08 '13 at 08:47

-1

I think we would need to see the report class, obviously you are handling well the stack so the problem must be at the report (stacks top) when you try to delete them.

Check/create the report destructor, there must be something that you might handle there too

answered Aug 08 '13 at 08:47

javier_domenech

5,995
6
37
59

I answered to this in a comment above: @claptrap well tought. Though there is no static memebres of pointers in report class. So it is not the problem. I answered my own question below. Thanks any way. Who voted the -1 on you ? This answer was not stupid at all... – Aug 08 '13 at 08:53

Cleaning up a stack of pointers

3 Answers3