dbirch997 / cantera

Automatically exported from code.google.com/p/cantera
1 stars 0 forks source link

Cantera/Python memory leak #55

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1. Create and "destruct" (by out-of-scope) a vast amount of Cantera Solution 
objects
2. Keep track of memory usage in e.g. top
3. See memory usage go up

What is the expected output? What do you see instead?
A non-increasing memory usage, since objects should be garbage collected when 
out of scope. I have not managed to find out if this is a python/C wrapper 
(refrence counting) issue or Cantera c++ issue.

What version of the product are you using? On what operating system?
svn co on sept. 5. 2011. Running on Fedora 15 (32 bit), sundials 2.3, numpy 
1.5.1, python 2.7. Have seen same behaviour on Centos 5.5 64-bit, and different 
python versions, numpy versions, sundials versions.

Please provide any additional information below.

Original issue reported on code.google.com by andr1...@gmail.com on 7 Sep 2011 at 6:52

Attachments:

GoogleCodeExporter commented 9 years ago
same behaviour for ThermoPhase objects

Original comment by andr1...@gmail.com on 7 Sep 2011 at 7:18

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
I have just checked on windows XP and python 2.6 and observed the same 
behaviour.

Original comment by andr1...@gmail.com on 8 Sep 2011 at 7:25

GoogleCodeExporter commented 9 years ago
I have done some further investigation.
1) I have taken one of the c++ demos and tailored it into something like my 
python example attached in the main post, and the memory usage was very stable. 
Ok, so this makes me think that the problem is either within in clib/python 
c-wrapper part.
2) I looked at the code in svn/cantera18/trunk/Cantera/clib/src/Storage.cpp and 
looked at the methods for deleting object pointers/id's in the storage vectors 
for transport, thermo and kinetics objects. the void Storage::deleteThermo 
never calls "delete" for the object id/pointer to be deleted from the storage 
vector. This puzzles me. Anyway I tried calling "delete", however to my 
disappointment it didn't help on memory usage. Even if delete is called, I can 
see by printing the id's of the gases I create, that the id count is ever 
increasing even if the pointers should be deleted. Last try was to call 
__thtable.pop_back(), this seemed to do the trick. I have tested this to see if 
it breaks anything elsewhere, but I don't think it should. Is there good reason 
for not using the pop_back() for deallocation? Maybe, this has already been 
tried and didn't do the job for some reason. Here's my rudimentary code (I am 
NOT a c/c++ programmer, so am I missing something obvious here?):

void Storage::deleteThermo(int n) {
    if (n == 0) return;
    if (n < 0 || n >= (int) __thtable.size())
        throw CanteraError("deleteThermo","illegal index");
    if (__thtable[n] != __thtable[0]){
        delete __thtable[n];
        __thtable.pop_back();
    }
    cout << n << endl;
}

Original comment by andr1...@gmail.com on 17 Sep 2011 at 10:35

GoogleCodeExporter commented 9 years ago
I am stupid. I just realised that pop_back() may delete the wrong objects, 
since it just removes the last element, maybe not element n

Original comment by andr1...@gmail.com on 17 Sep 2011 at 10:51

GoogleCodeExporter commented 9 years ago
Here's something that may work, at least memory usage is stable now. I must 
have missed a compile cycle, because I didn't see the immediate effects earlier

void Storage::deleteThermo(int n) {
    if (n == 0) return;
    if (n < 0 || n >= (int) __thtable.size())
        throw CanteraError("deleteThermo","illegal index");
    if (__thtable[n] != __thtable[0])
        delete __thtable[n];
    __thtable[n] = __thtable[0];
}

Original comment by andr1...@gmail.com on 17 Sep 2011 at 11:56

GoogleCodeExporter commented 9 years ago
Fixed in trunk as part of replacing Storage with Cabinet (see r1207).

Original comment by yarmond on 15 Mar 2012 at 8:16