search

alex-robinson / ncio

Simple Fortran interface to NetCDF reading and writing.
23 stars 9 forks source link

Small memory leak in test_ncio.f90 #3

Closed sebastianbeyer closed 9 years ago

sebastianbeyer commented 9 years ago

While searching for another bug in my code I found this. nc_dims is still a little leaky, but I don't know how fix this, since the dims and names are intent out, so we can not handle this in the subroutine. Anyway, we are talking about 100 bytes, so I am not sure if this is really worth the effort. ;-)

valgrind --leak-check=yes ./test_ncio.x                   
==29683== Memcheck, a memory error detector
==29683== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al.
==29683== Using Valgrind-3.10.1 and LibVEX; rerun with -h for copyright info
==29683== Command: ./test_ncio.x
==29683== 

 ====== WRITING ======

ncio:: nc_create   :: out_ncio.nc
ncio:: nc_write_map:: out_ncio.nc : polar_stereographic
ncio:: nc_write_dim:: out_ncio.nc : p                  1
ncio:: nc_write_dim:: out_ncio.nc : x                  8
ncio:: nc_write_dim:: out_ncio.nc : y                 10
ncio:: nc_write_dim:: out_ncio.nc : z                  6
ncio:: nc_write_dim:: out_ncio.nc : time               4
ncio:: nc_write_char:: warning: 2D character array could not be written: c2D

 ====== READING ======

 Title: Checkerboard output
 Institution: Universidad Complutense de Madrid; Potsdam Institute for Climate Impact Research
    x:         1.0     2.0     3.0     4.0     5.0     6.0     7.0     8.0
    y:         1.0     2.0     3.0     4.0     5.0     6.0     7.0     8.0     9.0    10.0
    z:         1.0     3.0     5.0     7.0     9.0    11.0
    time:      0.0     5.0   100.0   150.0
     c1D: testing   testing   testing   testing   testing   testing   testing   testing   

 ====== DONE ======

 dimnames= x                               y                               z                               
 dimlens=             8          10           6
==29683== 
==29683== HEAP SUMMARY:
==29683==     in use at exit: 963,528 bytes in 6 blocks
==29683==   total heap usage: 38,698 allocs, 38,692 frees, 53,213,179 bytes allocated
==29683== 
==29683== 12 bytes in 1 blocks are definitely lost in loss record 2 of 6
==29683==    at 0x4C29F90: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==29683==    by 0x4762E2: __ncio_MOD_nc_dims (ncio.f90:1132)
==29683==    by 0x48A357: MAIN__ (test_ncio.f90:167)
==29683==    by 0x48A6EE: main (test_ncio.f90:5)
==29683== 
==29683== 96 bytes in 1 blocks are definitely lost in loss record 4 of 6
==29683==    at 0x4C29F90: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==29683==    by 0x4767BA: __ncio_MOD_nc_dims (ncio.f90:1137)
==29683==    by 0x48A357: MAIN__ (test_ncio.f90:167)
==29683==    by 0x48A6EE: main (test_ncio.f90:5)
==29683== 
==29683== LEAK SUMMARY:
==29683==    definitely lost: 108 bytes in 2 blocks
==29683==    indirectly lost: 0 bytes in 0 blocks
==29683==      possibly lost: 0 bytes in 0 blocks
==29683==    still reachable: 963,420 bytes in 4 blocks
==29683==         suppressed: 0 bytes in 0 blocks
==29683== Reachable blocks (those to which a pointer was found) are not shown.
==29683== To see them, rerun with: --leak-check=full --show-leak-kinds=all
==29683== 
==29683== For counts of detected and suppressed errors, rerun with: -v
==29683== ERROR SUMMARY: 2 errors from 2 contexts (suppressed: 0 from 0)
[1]    29683 profile signal  valgrind --leak-check=yes ./test_ncio.x
alex-robinson commented 9 years ago

Good find, I added your deallocate commit to the master.

What do you suppose causes this memory leak in the nc_dims routine? I saw that it refers to the line when the names array is allocated for example. But I cannot see really what the problem would be. Unless Fortran is not robust concerning multiple optional allocatable arguments, or we are doing something wrong... Any thoughts?

sebastianbeyer commented 9 years ago

Yeah, names and dims are allocated, when they are given as optional arguments, but they are never deallocated. And it is impossible to deallocate them in this subroutine, because it it the purpose of this subroutine to set them. To solve this, one would need another subroutine, that is called after names and dims are no longer used and deallocates them. This would be a little bit cleaner, but maybe also confusing. I think this is not really an issue, because the memory that is allocated is so small.

alex-robinson commented 9 years ago

Ok, yes in that case I think it's not something to worry about. '''names''' and '''dims''' as variables defined in the main program, therefore it should be the responsibility of the user to deallocate them, in the same way they would deallocate anything else. But it's easy to overlook them...