Skip to content

Commit

Permalink
Fix memory leak in test "VegProdConstructor"
Browse files Browse the repository at this point in the history
- close #205

- remove leak suppression in "SW_VPD_construct" that is no longer needed

- the test "VegProdStructTest.VegProdConstructor" (previously named, as of v7.0.0, "VegTest.Constructor") now (1) uses a local copy of SW_VEGPROD and (2) calls `SW_VPD_deconstruct()` to avoid a memory leak

- Previously, the test used `SW_All.VegProd` or a global variable (for which `SW_VPD_construct()` has already been called once and allocated memory, e.g., during the test fixture's `SetUp()` or `Reset_SOILWAT2_after_UnitTest()`); then the (second) call to `SW_VPD_construct()` would allocate memory a second time (thus the memory previously allocated is leaked).
However, the call to `SW_VPD_deconstruct()` (either in the test or during the test fixture's `TearDown()` or `Reset_SOILWAT2_after_UnitTest()`) will de-allocate only the most recently allocated memory (and, if called again, was seeing only NULL).

- before this commit, I was getting with `MallocStackLogging=1 MallocStackLoggingNoCompact=1 MallocScribble=1 MallocPreScribble=1 MallocCheckHeapStart=0 MallocCheckHeapEach=0 leaks -quiet -atExit -- bin/sw_test`
> 7 leaks for 896 total leaked bytes.
    7 (896 bytes) << TOTAL >>
      1 (128 bytes) ROOT LEAK: 0x600000cb0100 [128]
      1 (128 bytes) ROOT LEAK: 0x600000cb0180 [128]
      1 (128 bytes) ROOT LEAK: 0x600000cb0200 [128]
      1 (128 bytes) ROOT LEAK: 0x600000cb0280 [128]
      1 (128 bytes) ROOT LEAK: 0x600000cb0300 [128]
      1 (128 bytes) ROOT LEAK: 0x600000cb0380 [128]
      1 (128 bytes) ROOT LEAK: 0x600000cb0400 [128]

- after this commit, I'm getting
> 0 leaks for 0 total leaked bytes.
  • Loading branch information
dschlaep committed Jul 18, 2023
1 parent e429d21 commit 5322ae0
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 11 deletions.
3 changes: 0 additions & 3 deletions .LSAN_suppr.txt
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,5 @@

# These are known leaks that await fixing

# https://github.com/DrylandEcology/SOILWAT2/issues/205
leak:SW_VPD_construct

# on Apple arm64: https://github.com/google/sanitizers/issues/1501
leak:realizeClassWithoutSwift
1 change: 1 addition & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
* Bugfixes
* Fix an error where a pointer was freed even though it was not allocated
(issue #356; @dschlaep).
* Fix memory leak in test of `SW_VPD_construct()` (issue #205; @dschlaep).

# SOILWAT2 v7.0.0
* This version produces nearly identical simulation output
Expand Down
36 changes: 28 additions & 8 deletions tests/gtests/test_SW_VegProd.cc
Original file line number Diff line number Diff line change
Expand Up @@ -114,18 +114,38 @@ namespace {
int k;

// Test the SW_VEGPROD constructor 'SW_VPD_construct'
TEST_F(AllTest, VegProdConstructor) {
SW_VPD_construct(&SW_All.VegProd, &LogInfo);
SW_VPD_init_run(&SW_All.VegProd, &SW_All.Weather,
&SW_All.Model, SW_All.Site.latitude, &LogInfo);
TEST_F(VegProdStructTest, VegProdConstructor) {
// This test requires a local copy of SW_VEGPROD to avoid a memory leak
// (see issue #205)
// -- If using `SW_All.VegProd` or a global variable
// (for which `SW_VPD_construct()` has already been called once, e.g.,
// during the test fixture's `SetUp()`), then this (second) call to
// `SW_VPD_construct()` would allocate memory a second time
// while `SW_VPD_deconstruct()` will de-allocate memory only once
// (the call to `SW_VPD_deconstruct()`during the test fixture's `TearDown()`
// would see only NULL and thus not de-allocate the required second time
// to avoid a leak)
SW_VEGPROD SW_VegProd;

SW_VPD_construct(&SW_VegProd, &LogInfo); // allocates memory

SW_VPD_init_run(
&SW_VegProd,
&SW_All.Weather,
&SW_All.Model,
SW_All.Site.latitude,
&LogInfo
);

ForEachVegType(k) {
EXPECT_DOUBLE_EQ(1., SW_All.VegProd.veg[k].co2_multipliers[BIO_INDEX][0]);
EXPECT_DOUBLE_EQ(1., SW_All.VegProd.veg[k].co2_multipliers[BIO_INDEX][MAX_NYEAR - 1]);
EXPECT_DOUBLE_EQ(1., SW_VegProd.veg[k].co2_multipliers[BIO_INDEX][0]);
EXPECT_DOUBLE_EQ(1., SW_VegProd.veg[k].co2_multipliers[BIO_INDEX][MAX_NYEAR - 1]);

EXPECT_DOUBLE_EQ(1., SW_All.VegProd.veg[k].co2_multipliers[WUE_INDEX][0]);
EXPECT_DOUBLE_EQ(1., SW_All.VegProd.veg[k].co2_multipliers[WUE_INDEX][MAX_NYEAR - 1]);
EXPECT_DOUBLE_EQ(1., SW_VegProd.veg[k].co2_multipliers[WUE_INDEX][0]);
EXPECT_DOUBLE_EQ(1., SW_VegProd.veg[k].co2_multipliers[WUE_INDEX][MAX_NYEAR - 1]);
}

SW_VPD_deconstruct(&SW_VegProd);
}


Expand Down

0 comments on commit 5322ae0

Please sign in to comment.