Vector Class: Language-based memory leak??

[IWishIKnew]

Alright, so I've noticed this before, but it was somwhat nominal so I dismissed it. Recently, however, it was impossible to notice: After calling a vector<string> which is a member of a class, when the scope of the class ends, and the memory/stack is "freed", there is surplus memory usage in the program.

I noticed this, only because it was magnified by roughly 500x. Lets me explain:

I am close to finishing a program that scans your entire filesystem on your hard-drive. Labeled as a 'snapshot', you can take another 'snapshot' and the program will note all changes to your filesystem. The user can see all folders/files created and folders/files deleted in between the times in which the snapshots were taken.

Since this requires a list of paths, I wrote a class to load the snapshots into a vector.

Now, this all went fine (roughly 400 Kb of memory taken up)... AND THEN I called a function that returned a reference to this vector. The program just about instantly took up 30,000 Kb, and when the scope of the class ended, and the program was somehow allocating between 3,000 and 7,000 Kb.

I managed to increase the memory efficiency as much as humanly possible. So much so that after the scope ended, the memory allocated for the program was not decreased. This line was at about 2,708Kb.

Great, so... nothing more I can do. I was to literally poping back the vector and useing the class as a pointer. That made significant differences.

I do not understand why this is, but mabey someone who knows more can explain this to me. I have noticed that in some of my other programs that use vectors heavily, small memory jumps happen. Since this "snapshot" program has to lod nearly 20,000+Kb of memory, this MUST be addressed.

I may post the code tomorrow, I am worn completely out right now... I spent about 4 hours tweaking the code to make it more efficient.

Anyway, thank you for your time.

[tntxtnt]

I believe the problem is in your code.

I have tried std::vector<bool> to sieve up to ~ 189 million numbers and it doesn't leak anything. I guess std::vector<string> shouldn't leak either.

https://dl.dropboxusercontent.com/u/93297896/prog10_sieve/vector_bool.png
https://dl.dropboxusercontent.com/u/93297896/prog10_sieve/freed2.png

Make sure that you have delete for every new, and delete [] for every new [] you use. That's all I know...

[TheIdeasMan]

Maybe you would be better off with a different container. I am sure you know what happens when a vector has to resize itself. I never use a vector for anything that is open ended in size.

Avoid this by using a std::list , std::map, std::set or others.

Investigate using smart pointers - then you won't have to ever worry about having to use new & delete ever again, use std::make_shared instead of new. A problem with delete is that you never know if something throws an exception somewhere, so delete never gets called. Smart pointers mean that objects will always be destructed.

http://www.cplusplus.com/reference/memory/shared_ptr/
http://www.cplusplus.com/reference/memory/make_shared/
http://www.cplusplus.com/reference/memory/unique_ptr/
http://www.cplusplus.com/reference/memory/auto_ptr/
http://www.cplusplus.com/reference/memory/weak_ptr/

HTH

[IWishIKnew]

Interesting. Thanks for the advice.

A map would actually suite my needs perfect in this scenario. I am not storing any pointers in the list, so I shouldn't have any problems with the delete call.

I will look into that make_shared declaration.

[IWishIKnew]

ok, I have modified it to use maps. Still a huge memory leak, but not nearly as big as before. declaring unique_ptr and shared_ptr seem to help, but theres still over 4,000 Kb left over...

Using unique_ptr was more effective though...

[ne555]

¿and your code?

> Using unique_ptr was more effective though...
If you don't plan to share ownership, then don't make it shared.
They do have a cost

Sorry, ¿why do you use pointers for?

[IWishIKnew]

Here is the source:

Snapshot_class.h: http://pastebin.com/59QDBhLQ

Snapshot_class.cpp: http://pastebin.com/LJhpc8eR

Mem leak emplementation: http://pastebin.com/mBnBAUYR

[ne555]

> AND THEN I called a function that returned a reference to this vector
you are returning a copy

	path_count = snapshot.gsnap().size(); //here is when memory skyrockets
//map<unsigned int, string> snapshot_class::gsnap() 

the temporary dies after that line, ¿how are you measuring the memory usage?

Try to provide a testable code.


I'll ask again, ¿why is snapshot_class::snapshot a pointer?

	snapshot_class snapshot = snap.path; //copying a shared pointer
	//path_count = snapshot.gsnap().size();
	snapshot.clear(); //after this, there is remaining memory allocated... 

well, snap.path.snapshot should still point to that memory, so it wouldn't be released

[IWishIKnew]

@ne

ne said:

¿why is snapshot_class::snapshot a pointer?

It isn't, look at the code.

ne said:

the temporary dies after that line, ¿how are you measuring the memory usage?

I know, but for some reason, it carries on, and I don't knwo why. I run task manager and monitor the memory usage through it, and it has over 40,000 Kb allocated until it's scope ends, or it is manually deallocated. After that, there is between 1,000 and 5,000 Kb left in leaked memory.

ne said:

well, snap.path.snapshot should still point to that memory, so it wouldn't be released

I don't think you quite read my code. In the class, the map is declared as a shared_ptr and when snapshot_class::clear() is called, that pointer is reset, clearing the memory.

snap is not a class, it is a data structure.

snapshot_class snapshot = snap.path; declares a class with the handle snapshot with snap.path as the argument for construction.

**********************************************************************
I'll explain a little more how the class is supposed to work:

the snapshot_class can be constructed with the following arguments:

const string&
const char*
[nothing]

when given a string it will assume that string is a file path and attempt to load it as a 'snapshot'. This fills snapshot_class::snapshot with a list of string that represent paths which make up a 'snapshot'. In my case, there are over 207,000 paths loaded, and this is not in administrative mode (so it would pick up more if run in admin mode).

when given no argument, it creates a snapshot. This recursively iterates through every file/folder, and directly writes them to a file.

When comparing 2 snapshots, the list will need to be retrieved from the class, but not modified. I also don't want it to be able to be modified.

When the data is retrieved, as in my example with the menu, it creates a memory leak.

[naraku9333]

You never answered the question, why are you using pointers?

Edit:
Its pretty hard to debug a program when it cant be compiled, you should either provide a smaller compilable code that reproduces the issue (preferred) or provide all source needed to compile. A shot in the dark try changing snapshot_class snapshot = snap.path; //no problems here to snapshot_class snapshot(snap.path);

[ne555]

@IWishIKn
shared_ptr<map<unsigned int, string>> snapshot; an smart pointer

I don't think you know what shared means.
The memory would be deallocated when nobody is pointing to it.

shared_ptr<int> p,q;
p = make_shared( new int(42) );
q = p;
p.clear();

`q' still points to a valid address, containing a 42.

There is no meaningful difference between a class or an struct

> declares a class with the handle snapshot with snap.path as the argument for construction.
¿what type is snap.path? My guess was that `snap.path' is an snapshop_class
to avoid confusion try to use explicit constructors.

> I also don't want it to be able to be modified.
return a const reference

[IWishIKnew]

ok, ne, you HAVE to read the code. snap is CLEARLY declared as an argument of the function.

[ne555]

me wrote:

¿why is snapshot_class::snapshot a pointer?

//snapshot_class.h
class snapshot_class{
//...
   shared_ptr<map<unsigned int, string>> snapshot;
};

Also, you never provide basic_snap_data definition
int snapshot_menu(const basic_snap_data& snap, const int& num_of_snaps)

[cire]

I run task manager and monitor the memory usage through it, and it has over 40,000 Kb allocated until it's scope ends, or it is manually deallocated. After that, there is between 1,000 and 5,000 Kb left in leaked memory.

The task manager is not suitable for the task you're using it for.

[IWishIKnew]

@ cire:

hmm. Is there anything else I can use, then?

[naraku9333]

IWishIKnew wrote:

This is the last time I'm assisting you.

I suspect you will hear that a lot your self with that attitude.

Edit: You have been asked multiple times why you are using pointers and have yet to answer. Do you even know what causes memory to leak? You have also been asked to provide testable code that reproduces the issue. If you can't be bothered to help, why should we?

[Disch]

@naraku9333:

I was just going to make a very similar reply before I saw your edit.

[tntxtnt]

and why do you use snapshot like a static member?

shouldn't it be
this->filename
this->date
this->id
this->snapshot

instead of
snapshot_class::filename
snapshot_class::date
snapshot_class::id
snapshot_class::snapshot
?

[Disch]

@tntxtnt:

That's just stylistic. In this case:

filename = x;

this->filename = x;

snapshot_class::filename = x;

Are all equivalent statements.

[tntxtnt]

lol after all these years I thought this->data and class::data are different. Thanks Disch.