[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Memory leak in netcdf_c++ version 3.3

Hi Mike,

>   I have found (and I think fixed) a memory leak in the netcdf C++
> library (version 3.3).  It seems that all of the
> NcTypedComponent::as_*() functions were getting a pointer to a new
> NcValues but not deleting it.  The patch below fixes this problem.
> Purify seems to think that the memory leak is now fixed.
> 
> 
> Mike Romberg (address@hidden)
> 
> --- netcdf-3.3/src/cxx/netcdf.cc      Mon Apr 21 12:28:25 1997
> +++ netcdf.cc Wed Jun 18 11:11:16 1997
> @@ -428,10 +428,13 @@
>  }
>  
>  #define Nc_as(TYPE) name2(as_,TYPE)
> -#define NcTypedComponent_as(TYPE)                                          \
> -TYPE NcTypedComponent::Nc_as(TYPE)( long n ) const                         \
> -{                                                                          \
> -    return values()->Nc_as(TYPE)(n);                                       \
> +#define NcTypedComponent_as(TYPE)                                      \
> +TYPE NcTypedComponent::Nc_as(TYPE)( long n ) const                     \
> +{                                                                      \
> +  NcValues *tmp = values();                                               \
> +  TYPE rval = tmp->Nc_as(TYPE)(n);                                        \
> +  delete tmp;                                                             \
> +  return rval;                                                            \
>  }
>  NcTypedComponent_as(ncbyte)
>  NcTypedComponent_as(char)
> @@ -443,7 +446,10 @@
>  
>  char* NcTypedComponent::as_string( long n ) const
>  {
> -    return values()->as_string(n);
> +    NcValues *tmp = values();
> +    char *rval = tmp->as_string(n);
> +    delete tmp;
> +    return rval;
>  }
>  
>  NcTypedComponent::NcTypedComponent ( NcFile* nc )

Thanks!

In researching the problem, I discovered that it was also reported by
Tomas Johannesson in 1995, but we somehow misplaced the bug report and
didn't fix it.  I've appended his description, in case you're interested.

After testing your patch, I've added a description of the bug and a link
to your patch on the "Known Problems with the netCDF 3.3.1 Distribution"
page at

   http://www.unidata.ucar.edu/packages/netcdf/known_problems.html

Thanks again for providing the patch.

--Russ

_____________________________________________________________________

Russ Rew                                         UCAR Unidata Program
address@hidden                     http://www.unidata.ucar.edu

Replied: Mon, 04 Mar 1996 09:07:38 -0700
Replied: ""Tomas Johannesson" <address@hidden> "
Subject: more about the c++ interface
To: address@hidden
Date: Sat, 7 Oct 1995 12:34:54 +0000 (GMT)
From: "Tomas Johannesson" <address@hidden>
X-Mailer: ELM [version 2.4 PL23]
Mime-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Transfer-Encoding: 8bit
Content-Length: 2933      

Hello again Russ,

I have just written an application which makes use of the c++
interface to NetCDF and writes measurements to portable files
which can be moved between computers.
NetCDF works fine and I am pleased with the performance and the
c++ interface. I think I have found an error the "values()" and
the "as_*()" functions of NcTypedComponet part of the c++ interface
(which I therefore avoided in my application).

The problem I see with these functions is that the c++-info says
about the values() function of NcTypedComponet that it:
"Returns a pointer to the block of all values for the variable or
 attribute.  This will be deleted when the variable or attribute is
 destroyed."
It appears to me that this is not correct. Each call to values
used get_space() to allocate new space for the values and return
a pointer to the newly allocated space (filled with the values
of the variable). Repeated calls to values() return different
pointers and the NcTypedComponet variable does not keep track
of the space it has allocated. This space will therefore not
be deleted when the NcTypedComponet is deleted.
Furthermore, it seems to me that each call to the as_char(int n),
as_short(int n), etc. functions makes an independent call to values()
through the "return values()->Nc_as(TYPE)(n);" expression.
The pointer returned by "values" in this case (different pointer
and new space for each call?) disappears and can never be deleted
by the application. Finally, why do these functions use an "int"
in their arguments rather than "long" which is used for indexing
everywhere else in NetCDF?

I think each NcTypedComponet should have a pointer which is initially
zero and potentially points to all the values of this variable
if the user has called values() or any of the as_*() functions.
The pointer should only be assigned new values the first time
these functions are called and there should be an explicit
function which the user can call to release the values if
he so chooses after he/she has used them (i.e. a release_values()
function) which sets the pointer to zero after deleting the values
vector. This pointer should be deleted when the variable itself is
deleted if it is non-zero at that point in time.
In this way the description in the info file will be correct.

Finally I want to say that I appreciate the effort that you
at UCAR have put into NetCDF and the continuing work to improve
the library and make it more useful for the general public.
I have found it very useful and I am especially impressed with
the amount of work you have put into the documentation and
description of the library which is the part of development
work of this kind that is ofter neglected.

best wishes, tomas

-- 
  Tomas Johannesson                   Electronic mail: address@hidden
  Orkustofnun (National Energy Authority)
  Grensasvegi 9, IS-108 Reykjavik, Iceland
  Phone: +354-5696041   Fax: +354-5688896   Home: 354-5535639