Re: [patch, libfortran] PR40508 Memory Leak

[Jerry DeLisle <jvdelisle at verizon dot net>]

Dominique Dhumieres wrote:
> Jerry,
>
> AFAICT your patch in http://gcc.gnu.org/ml/fortran/2009-06/msg00233.html
> fixes the memory leak.
>
> The diff between my 4.4.1 (noleak) and patched trunk is:

--- snip ---

> @@ -164,7 +169,11 @@ save_parsed_format (st_parameter_dt *dtp
>      free_format_data (u->format_hash_table[hash].hashed_fmt);
>    u->format_hash_table[hash].hashed_fmt = NULL;
>  
> -  u->format_hash_table[hash].key = dtp->format;

The reason for the leak is the unit structure including the hash table is 
initialized to NULL each time it is created for each I/O statement when it 
involves an internal unit. That means this IF statement is always false and 
therefore no memory is ever released by this line of code.  In fact, at this 
point, the pointer u, is not the same as when the key was allocated.

> +  if (u->format_hash_table[hash].key != NULL)
> +    free_mem (u->format_hash_table[hash].key);
> +  u->format_hash_table[hash].key = get_mem (dtp->format_len);
> +  memcpy (u->format_hash_table[hash].key, dtp->format, dtp->format_len);
> +
>    u->format_hash_table[hash].key_len = dtp->format_len;
>    u->format_hash_table[hash].hashed_fmt = dtp->u.p.fmt;
>  }
> @@ -1209,7 +1218,8 @@ parse_format (st_parameter_dt *dtp)
>        free_format_hash_table (dtp->u.p.current_unit);
>        return;
>      }

The following code prevents the allocation from ever happening and therefore 
fixes it.
> -  save_parsed_format (dtp);
> +  if (!is_internal_unit (dtp))
> +    save_parsed_format (dtp);
>  }
>
> It would be interesting to understand why there is no leak with the - variant.

Putting it another way.  The caching depends on the gfc_unit structure being 
persistent.  The gfc_unit structure for internal units is not persistent. 
Internal unit structures live only for the life of a single I/O statement. (Yes, 
this is fertile ground for further optimization.)

I hope this is clear.  Richard threw the hint out at me on IRC. I think he was 
chuckling.  From the log:

(11:23:24 AM) richi: 390  memset (iunit, '\0', sizeof (gfc_unit));

Regards,

Jerry