-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Linux machines have indirect memory leak errors #205
Comments
- addressing #205 - added call to `SW_CTL_clear_model` before re-initializing SOILWAT2 - this is similar behavior to updated functions `start` of `rSOILWAT2` and `SXW_Reset` of `STEPWAT2`
I tried to replicate with
It throws a segmentation fault at the first death test. I wonder whether the combination of macOS X, valgrind, and DEATH gtests doesn't work. Before the segfault, I see a whole lot of
|
Yes, it did not fix the issue entirely but it did make it significantly better as with master I get: and in the new branch after the commit I'm getting: |
Nice; this is a good improvement! Could you please post (or send by email) what the remaining leaks are? |
|
Thanks for posting. Lines 636 and 639 of function
This memory should be freed by function
@nip5 Any idea what I am missing? |
Not from what I can see, |
It doesn't look like the code ever enters the line 127 conditional in mymemory.c, so p is never freed in memory. I think that may be the problem:
|
Hm, I'm not sure. The code lines |
But isn't it allocated outside of the ifdefs, so it always allocates but only sometimes deallocates? For instance it is allocated here:
|
The function For instance, Where do you see missing deallocations? |
|
I don't understand your question because The function Please describe your point/question in greater detail. |
Your last point is what I meant, in that p is allocated onto the heap so we need free it and I didn't see that being done. I saw that 'SW_VegProd.p_oagg [pd]' is being freed but I missed that 'SW_VegProd.p_oagg [pd]' is set as the returned p in the contruction function. |
Ok, good. So you agree with my earlier comment (#205 (comment)) that it is not obvious why the address sanitizer detects a leak in |
Yes, they look balanced to me. |
Do you have any other idea/suggestion what to look at for this issue? If not, we just let it sit for a while and revisit at some later time? Thanks for your work and interactions over the last days to solve several memory issues!! |
I did have an idea but I just investigated it and it wasn't the problem either! The memory issues don't crash the program now even with the flag set to address sanitize on so we could revisit it at a later date. No problem, every time I have to try and work through stuff like this I end up grasping concepts better so I enjoy it. |
Merge progress made so far on branch |
This shows up on
and
|
- use suppression file on `travis-ci` for severe testing target - address #205 (but not fixed)
To exclude known memory leaks from severe testing without disabling leak detection altogether: This reports on
|
another way to see this memory leak
|
Feature memory fixes - fix memory issues (close #205, close #356) - `SW_SIT_init_run()` does not need argument `PATH_INFO *PathInfo` - Reorganized tests - only use test fixtures where really needed - do not use test fixtures in death tests - new class "AllTestStruct" that behaves just like "AllTestFixture" but does not inherit from `::testing::Test` -- which can be used in death tests - place all relevant code inside `EXPECT_DEATH` in a compound statement -- code will otherwise be run twice (including the creation of test fixtures or constructing of a local copy of "AllTestStruct") - tool "check_SOILWAT2.sh" now runs additional steps with the `leaks` command
Development for v7.1.0 * Simulation output remains the same as the previous version. * Prepare for SOILWAT2 to become thread-safe and reentrant (#346; @N1ckP3rsl3y) * Definition clarifications * Thread-safe - Multiple threads (a future SOILWAT2 development) will not influence each other unintentionally. Here, we implemented structures to enable thread-local storage, i.e., each thread operates on local data structures or with static data. * Reentrant - The ability to correctly execute any part of the program by multiple threads simultaneously but independently from others. * All non-static variables are replaced by local variables; functions gained arguments to pass local variables by reference (replacing global variables). * New main abstract types * SW_ALL - Contains the existing structures that are relevant for the simulation itself, e.g., SW_MODEL, SW_SOILWAT, SW_WEATHER. * PATH_INFO - Holds information about location of input and output data, e.g., directories, file paths to input data. * SW_OUTPUT_POINTERS - Points to requested output subroutines. * LOG_INFO - Manages information for logging warnings and errors. * Tests now require `c++14` and utilize `googletest` `v1.14.0` (issue #339). * 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). Changes to inputs * The output separator `OUTSEP` has been unused since the switch (`v4.0.0`) from writing free-form text files to `"csv"` spreadsheets (where the separator is fixed to a comma); the occurrence of `OUTSEP` in `"outsetup.in"` is now deprecated and triggers a warning.
Linux machines have indirect memory leak errors when the environment variable
ASAN_OPTIONS='detect_leaks=1'
. Thus the temporary solution is to instead run with that variable set to0
. An instance of these errors:The text was updated successfully, but these errors were encountered: