Possible Memory Leak in io::readLine()?

[worky68]

I have been experimenting with C3 by porting the interpreter from Crafting Interpreters to it. I noticed when I run valgrind against my interpreter code that I see that two allocs were not freed in the temp allocator block of io::readline.

Here is the relevant portion of my code (I believe).

fn void repl() {

  for(;;) {
    io::printf("μ ");
    String line = io::readline()!!;
    defer line.free();

    if(line == "exit") {
      return;
    }

    vm::interpret(line);

  }
}

fn int main(String[] args) {
  vm::initVm();
  defer vm::freeVm();

  if(args.len == 1) {
    repl();
  } 
  else if(args.len == 2) {
    runFile(args[1]);
  } 
  else {
    io::fprintf(io::stderr(), "Usage: mush [path]\n")!!;
    return 64;
  }

  return 0;
}

And here is valgrind output.

$ valgrind --leak-check=full --show-leak-kinds=all ./build/mushd 
==279410== Memcheck, a memory error detector
==279410== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==279410== Using Valgrind-3.18.1 and LibVEX; rerun with -h for copyright info
==279410== Command: ./build/mushd
==279410== 
μ 1 + 2
== code ==
0000 1 OP_CONSTANT      0 '1'
0002   | OP_CONSTANT      1 '2'
0004   | OP_ADD
0005   | OP_RETURN
          
0000 1 OP_CONSTANT      0 '1'
          [ 1 ]
0002   | OP_CONSTANT      1 '2'
          [ 1 ][ 2 ]
0004   | OP_ADD
          [ 3 ]
0005   | OP_RETURN
3
μ exit
==279410== 
==279410== HEAP SUMMARY:
==279410==     in use at exit: 524,368 bytes in 2 blocks
==279410==   total heap usage: 10 allocs, 8 frees, 526,547 bytes allocated
==279410== 
==279410== 262,184 bytes in 1 blocks are still reachable in loss record 1 of 2
==279410==    at 0x4848899: malloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==279410==    by 0x14385F: std.core.mem.allocator.LibcAllocator.acquire (libc_allocator.c3:37)
==279410==    by 0x14812B: malloc_try (mem_allocator.c3:68)
==279410==    by 0x14812B: alloc_with_padding (mem_allocator.c3:236)
==279410==    by 0x14812B: std.core.mem.allocator.new_temp_allocator (temp_allocator.c3:40)
==279410==    by 0x148376: create_default_sized_temp_allocator (mem_allocator.c3:370)
==279410==    by 0x148376: std.core.mem.allocator.init_default_temp_allocators (mem_allocator.c3:393)
==279410==    by 0x111923: temp (mem_allocator.c3:386)
==279410==    by 0x111923: @pool (mem.c3:514)
==279410==    by 0x111923: readline (io.c3:69)
==279410==    by 0x111923: mush.repl (main.c3:16)
==279410==    by 0x111E87: mush.main (main.c3:43)
==279410==    by 0x11243B: @main_to_int_main_args (main_stub.c3:47)
==279410==    by 0x11243B: main (main.c3:38)
==279410== 
==279410== 262,184 bytes in 1 blocks are still reachable in loss record 2 of 2
==279410==    at 0x4848899: malloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==279410==    by 0x14385F: std.core.mem.allocator.LibcAllocator.acquire (libc_allocator.c3:37)
==279410==    by 0x14812B: malloc_try (mem_allocator.c3:68)
==279410==    by 0x14812B: alloc_with_padding (mem_allocator.c3:236)
==279410==    by 0x14812B: std.core.mem.allocator.new_temp_allocator (temp_allocator.c3:40)
==279410==    by 0x148451: create_default_sized_temp_allocator (mem_allocator.c3:370)
==279410==    by 0x148451: std.core.mem.allocator.init_default_temp_allocators (mem_allocator.c3:394)
==279410==    by 0x111923: temp (mem_allocator.c3:386)
==279410==    by 0x111923: @pool (mem.c3:514)
==279410==    by 0x111923: readline (io.c3:69)
==279410==    by 0x111923: mush.repl (main.c3:16)
==279410==    by 0x111E87: mush.main (main.c3:43)
==279410==    by 0x11243B: @main_to_int_main_args (main_stub.c3:47)
==279410==    by 0x11243B: main (main.c3:38)
==279410== 
==279410== LEAK SUMMARY:
==279410==    definitely lost: 0 bytes in 0 blocks
==279410==    indirectly lost: 0 bytes in 0 blocks
==279410==      possibly lost: 0 bytes in 0 blocks
==279410==    still reachable: 524,368 bytes in 2 blocks
==279410==         suppressed: 0 bytes in 0 blocks
==279410== 
==279410== For lists of detected and suppressed errors, rerun with: -s
==279410== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

[lerno]

No, that's just the temp allocators I believe. Readline just happens to use the temp allocators and they're lazily allocated. Recently I added a finalizer to drop the temp allocators at exit, so you should probably not see those anymore. That said, I do encourage using the temp allocators, and the repl code is ideal for it:

for(;;) {
  @pool() {
    io::printf("μ ");
    String line = io::treadline()!!; // Temp allocated

    if(line == "exit") {
      return;
    }

    vm::interpret(line);
  
  }; // Everything temp allocated inside is released!
}

So as you see, this lets you avoid the need to explicitly free.

[worky68]

Thank you! I am enjoying C3 a lot.

[lerno]

I happy to hear that!

[barunslick]

Is there a way in C3 we can reuse our own buffer inside the loop of repl instead of using temporary allocators? Something like getline in C. What would be recommended way to do it?

[lerno]

@barunslick there are several options. The simplest one would be:

char[200] buffer;
ArenaAllocator arena;
arena.init(buffer);
String line = io::readline(allocator: &arena)!!;

Of course, this one would panic if more than 200 bytes were to be allocated. A safe thing would be to use @stack_mem which uses data on the stack, but will go to the heap allocator if needed:

@stack_mem(200; Allocator mem)
{
   String line = io::readline(allocator: mem)!!;
   ...
}; // Memory is released here.

[lerno]

But you can also just consider writing your own custom function for it.

[barunslick]

Thank you 🙇