Re: [Swig-devel] memory leak in SWIG_AsPtr_std_string?

[William S F. <ws...@fu...>]

Will Nolan wrote:
> When testing I noticed that a const std::string ref passed in as an argument to a C++ member function resulted in a memory leak.
> 
> Looking at the code 1.3.40, it seems the problem comes from here:
> 
> SWIG_AsCharPtrAndSize(SEXP obj, char** cptr, size_t* psize, int *alloc)
> {
>   if (cptr && Rf_isString(obj)) {
>     const char *cstr = CHAR(STRING_ELT(obj, 0));
>     int len = strlen(cstr);
> 
>     if (alloc) {
>       if (*alloc == SWIG_NEWOBJ) {
>         *cptr = reinterpret_cast< char* >(memcpy((new char[len + 1]), cstr, sizeof(char)*(len + 1)));
>         *alloc = SWIG_NEWOBJ;
>       } else {
>         *cptr = reinterpret_cast< char * >(malloc(len + 1));
>         *cptr = strcpy(*cptr, cstr);
>         *alloc = SWIG_OLDOBJ;
> 
> <snip>
> 
> The code in std_strings.swg in Lib/typemaps does the following:
> 
> %define %std_string_asptr(String, Char, SWIG_AsCharPtrAndSize, Frag)
> %fragment(SWIG_AsPtr_frag(String),"header",fragment=Frag) {
> SWIGINTERN int
> SWIG_AsPtr_dec(String)(SWIG_Object obj, String **val) 
> {
>   Char* buf = 0 ; size_t size = 0; int alloc = SWIG_OLDOBJ;
>   if (SWIG_IsOK((SWIG_AsCharPtrAndSize(obj, &buf, &size, &alloc)))) {
>     if (buf) {
>       if (val) *val = new String(buf, size - 1);
>       if (alloc == SWIG_NEWOBJ) %delete_array(buf);
>       return SWIG_NEWOBJ;
>     } else {
>       if (val) *val = 0;
>       return SWIG_OLDOBJ;
>     }
> 
> <snip>
> 
> SWIG_AsPtr_std_string (which is generated from this) calls SWIG_AsCarPtrAndSize with alloc = SWIG_OLDOBJ, which leads us through the part of the if-else clause that does the malloc (not the new[]).  Then, a new std::string is allocated with new and initialized with the contents of that buffer.  However, after this is the line "if (alloc == SWIG_NEWOBJ) %delete_array(buf);" -- so buf is never freed for the SWIG_OLDOBJ case, which is always the case.
> 
> Changing the code to the following:
> 
> %define %std_string_asptr(String, Char, SWIG_AsCharPtrAndSize, Frag)
> %fragment(SWIG_AsPtr_frag(String),"header",fragment=Frag) {
> SWIGINTERN int
> SWIG_AsPtr_dec(String)(SWIG_Object obj, String **val) 
> {
>   Char* buf = 0 ; size_t size = 0; int alloc = SWIG_OLDOBJ;
>   if (SWIG_IsOK((SWIG_AsCharPtrAndSize(obj, &buf, &size, &alloc)))) {
>     if (buf) {
>       if (val) *val = new String(buf, size - 1);
>       if (alloc == SWIG_NEWOBJ) %delete_array(buf); else free(buf);
>       return SWIG_NEWOBJ;
>     } else {
>       if (val) *val = 0;
>       return SWIG_OLDOBJ;
>     }
> 
> <snip>
> 
> Does the right thing and the memory leak goes away.  (side note: this all seems kind of ugly)
> 
> Before I go barking up the wrong tree, am I completely off base here?  This appears to be a genuine memory leak that could apply to all languages that pass strings to functions that take std::string's as arguments.
> 
Your analysis looks correct except the leak is probably only in R. I 
think you should base the solution on one of the other UTL language's 
implementation of SWIG_AsCharPtrAndSize, eg in perlstrings.swg and 
pystrings.swg.

Try the attached patch and if it solves the problem, I'll commit it.

William