Fix many (but not all) memory leaks using gcc -fsanitize=address #1173
Conversation
#1168 #1163 #1162 This PR partially fixes memory leaks in the netcdf-c library, in the ncdump utility, and in some test cases. The netcdf-c library now runs memory clean with the assumption that the --disable-utilities option is used. The primary remaining problem is ncgen. Once that is fixed, I believe the netcdf-c library will run memory clean with no limitations. Notes ----------- 1. Memory checking was performed using gcc -fsanitize=address. Valgrind-based testing has yet to be performed. 2. The pnetcdf, hdf4, and examples code has not been tested. Misc. Non-leak changes 1. Make tst_diskless2 only run when netcdf4 is enabled (issue 1162) 2. Fix CmakeLists.txt to turn off logging if ENABLE_NETCDF_4 is OFF 3. Isolated all my debug scripts into a single top-level directory called debug 4. Fix some USE_NETCDF4 dependencies in nc_test and nc_test4 Makefile.am
|
This pull request fixes 1 alert when merging 245961d into 8d5e66e - view on LGTM.com fixed alerts:
Comment posted by LGTM.com |
| # Build test programs plus programs used in test scripts. | ||
| check_PROGRAMS = $(NC4_TESTS) renamegroup tst_empty_vlen_unlim | ||
| TESTS = $(NC4_TESTS) | ||
| if USE_NETCDF4 |
There was a problem hiding this comment.
This if USE_NETCDF4 is not needed. The nc_test4 directory will be skipped if netCDF-4 is not enabled, so there is no need to check USE_NETCDF4, it will always be true.
There was a problem hiding this comment.
Thanks for pointing out the redundancy; reviewing this now then will get the PR out.
There was a problem hiding this comment.
Rather than kick off the whole CI chain by removing this redundancy, I'll note it for removal after the merge.
|
@DennisHeimbigner I'm seeing a failure, repeatable, in dap4_test/test_remote.sh. The failure details can be seen here: |
|
|
The issue is as follows on OSX; I'm not sure why The file is In the Ignoring whitespace via diff on OSX doesn't appear to fix this issue, as it does on Linux. |
|
Added the fix, baby-sitting the CI, will merge and update other PR's ASAP. New RC tomorrow. |
|
This pull request fixes 1 alert when merging 5ad2410 into 8d5e66e - view on LGTM.com fixed alerts:
Comment posted by LGTM.com |
|
ok. |
|
Merging into release branch for the RC going out today; it will then make its way back into the |
This is a follow up to PR #1173 Sorry that it is so big, but leak suppression can be complex. This PR fixes all remaining memory leaks -- as determined by -fsanitize=address, and with the exceptions noted below. Unfortunately. there remains a significant leak that I cannot solve. It involves vlens, and it is unclear if the leak is occurring in the netcdf-c library or the HDF5 library. I have added a check_PROGRAM to the ncdump directory to show the problem. The program is called tst_vlen_demo.c To exercise it, build the netcdf library with -fsanitize=address enabled. Then go into ncdump and do a "make clean check". This should build tst_vlen_demo without actually executing it. Then do the command "./tst_vlen_demo" to see the output of the memory checker. Note the the lost malloc is deep in the HDF5 library (in H5Tvlen.c). I am temporarily working around this error in the following way. 1. I modified several test scripts to not execute known vlen tests that fail as described above. 2. Added an environment variable called NC_VLEN_NOTEST. If set, then those specific tests are suppressed. This should mean that the --disable-utilities option to ./configure should not need to be set to get a memory leak clean build. This should allow for detection of any new leaks. Note: I used an environment variable rather than a ./configure option to control the vlen tests. This is because it is temporary (I hope) and because it is a bit tricky for shell scripts to access ./configure options. Finally, as before, this only been tested with netcdf-4 and hdf5 support.
re: github issues
#1168
#1163
#1162
This PR partially fixes memory leaks in the netcdf-c library,
in the ncdump utility, and in some test cases.
The netcdf-c library now runs memory clean with the assumption
that the --disable-utilities option is used. The primary remaining
problem is ncgen. Once that is fixed, I believe the netcdf-c library
will run memory clean with no limitations.
Notes
Valgrind-based testing has yet to be performed.
Misc. Non-leak changes
called debug