Cantera / cantera

Chemical kinetics, thermodynamics, and transport tool suite
https://cantera.org
Other
624 stars 350 forks source link

GRI30 Memory Leak in MATLAB Function #1734

Closed cboushehri closed 4 months ago

cboushehri commented 4 months ago

Problem description

When a MATLAB function that instantiates a GRI30 object returns, some memory is not freed. Running such a function multiple times will quickly crash the system.

Steps to reproduce

  1. Create MATLAB function that calls GRI30:
function leaky()
     leak = GRI30;
end
  1. Call function in a loop:
    for i = 1:1000
    leaky();
    end

Behavior

Upon running the for loop, memory rapidly fills up until the system crashes. Pressing ctrl-C during execution of the for loop stops the leak. Subsequently running clear or clear all does not free the memory. Quitting the MATLAB instance does free the memory.

System information

Reproduced on three machines: Cantera 2.6.0, MATLAB R2021a, Windows 7. Cantera 3.0.0, MATLAB R2022b, Windows 10. Cantera 3.0.0, MATLAB R2022b, Windows 10.

ischoegl commented 4 months ago

Thanks for reporting - this looks like the destructor isn't working as expected: there really only should be a single copy of GRI30 - or the associated Solution - at any given time.

Given that the MATLAB toolbox is already removed for the next release of Cantera (via #1670), this will not be fixed. An experimental replacement toolbox is already included in Cantera 3.0, which will eventually replace the old toolbox (although not necessarily in 3.1).

PS: given the age of the toolbox, the bug is likely in the MATLAB/MEX interface. At the same time, it could go back to clib, which is an actively supported part - note to maintainers: this needs to be checked.

speth commented 4 months ago

On the current main branch, the following works fine (added in test_clib.cpp) with memory usage stabilizing around 14 MB:

TEST(ct, memleak)
{
    for (size_t i = 0; i < 1000; i++) {
        int thermo = thermo_newFromFile("gri30.yaml", "gri30");
        int kin = kin_newFromFile("gri30.yaml", "", thermo, -1, -1, -1, -1);
        int tran = trans_newDefault(thermo, 0);
        kin_del(kin);
        trans_del(tran);
        thermo_del(thermo);
    }
}

which suggests that this was a problem specifically with the old Matlab toolbox, not with the clib interface.

ischoegl commented 3 months ago

While #1754 resolves some memory leaks in clib, those are not responsible for the behavior reported above:

In the experimental MATLAB toolbox, the following modification is necessary to run the test:

function leaky()
     leak = Solution('gri30.yaml', 'gri30', 'none');
end

where no crashes are observed.