Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Leak caused by fread

I'm profiling code of a game I wrote and I'm wondering how it is possible that the following snippet causes an heap increase of 4kb (I'm profiling with Heapshot Analysis of Xcode) every time it is executed:

u8 WorldManager::versionOfMap(FILE *file)
{
  char magic[4];
  u8 version;

  fread(magic, 4, 1, file); <-- this is the line
  fread(&version,1,1,file);
  fseek(file, 0, SEEK_SET);

  return version;
}

According to the profiler the highlighted line allocates 4.00Kb of memory with a malloc every time the function is called, memory which is never released. This thing seems to happen with other calls to fread around the code, but this was the most eclatant one.

Is there anything trivial I'm missing? Is it something internal I shouldn't care about?

Just as a note: I'm profiling it on an iPhone and it's compiled as release (-O2).

like image 726
Jack Avatar asked Apr 14 '12 17:04

Jack


1 Answers

If what you're describing is really happening and your code has no bugs elsewhere, it is a bug in the implementation, I think.

More likely I think, is the possibility that you don't close the file. Stdio streams use buffering by default if the device is non-interactive, and the buffer is allocated either at the time the file is opened or when I/O is performed. While only one buffer should be allocated, you can certainly leak the buffer by forgetting to close the file. But certainly, closing the file should free the buffer. Don't forget to check the value returned by fclose.

Supposing for the sake of argument that you are correctly closing the file there are a couple of other nits in your code which won't be causing this problem but I'll mention anyway.

First your fread call reads an object having one member of size 4. You actually have an object having 4 members of size 1. In other words the numeric arguments to fread are swapped. This makes a difference only in the meaning of the return value (important in the case of a partial read).

Second, while your first call to fread correctly hard-codes the size of char as 1 (in C, that is the definition of 'size'), it's probably better stylistically to use sizeof(u8) in the second call to fread.

If the idea that this really is a memory leak is a correct interpretation (and there aren't any bugs elsewhere) then you may be able to work around the problem by turning off the stdio buffering for this particular file:

bool WorldManager::versionOfMap(FILE *file, bool *is_first_file_io, u8 *version)
{
  char magic[4];
  bool ok = false;
  if (*is_first_file_io) 
  {
    // we ignore failure of this call
    setvbuf(file, NULL, _IONBF, 0);
    *is_first_file_io = false;
  }

  if (sizeof(magic) == fread(magic, 1, sizeof(magic), file) 
      && 1 == fread(version, sizeof(*version), 1, file))
  {
      ok = true;
  }
  if (-1 == fseek(file, 0L, SEEK_SET))
  {
      return false;
  }
  else
  {
      return ok && 0 == memcmp(magic, EXPECTED_MAGIC, sizeof(magic));
  }
}

Even if we're going with the hypothesis that this really is a bug, and the leak is real, it is well worth condensing your code to the smallest possible example that still demonstrates the problem. If doing that reveals the true bug, you win. Otherwise, you will need the minimal example to report the bug in the implementation.

like image 190
James Youngman Avatar answered Sep 27 '22 19:09

James Youngman