Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Memory leak when using fs.readAllAlloc multiple times #4656

Closed
leroycep opened this issue Mar 6, 2020 · 1 comment
Closed

Memory leak when using fs.readAllAlloc multiple times #4656

leroycep opened this issue Mar 6, 2020 · 1 comment
Labels
bug Observed behavior contradicts documented or intended behavior standard library This issue involves writing Zig code for the standard library.
Milestone

Comments

@leroycep
Copy link
Contributor

leroycep commented Mar 6, 2020

Not sure if this is a bug in the LeakCountAllocator, or readAllAlloc, but the following code will leak an allocation:

test "readAllAlloc memory leak" {
    const cwd = fs.cwd();
    try cwd.writeFile("test.txt", "Hello, world!\n");
    const file = try cwd.openFile("test.txt", .{ .read = true });

    const file_contents = try file.inStream().stream.readAllAlloc(testing.allocator, 50);
    testing.allocator.free(file_contents);

    const file_contents2 = try file.inStream().stream.readAllAlloc(testing.allocator, 50);
    testing.allocator.free(file_contents2);
}

The following output is generated when using zig test:

test "readAllAlloc memory leak"...error - detected leaked allocations without matching free: 1

/home/leroycep/sources/github.com/ziglang/zig/lib/std/special/test_runner.zig:52:46: 0x243b78 in std.special.main (test)
                error.Leak => std.debug.panic("", .{}),
                                             ^
/home/leroycep/sources/github.com/ziglang/zig/lib/std/start.zig:256:37: 0x20a36b in std.start.posixCallMainAndExit (test)
            const result = root.main() catch |err| {
                                    ^
/home/leroycep/sources/github.com/ziglang/zig/lib/std/start.zig:120:5: 0x20a19f in std.start._start (test)
    @call(.{ .modifier = .never_inline }, posixCallMainAndExit, .{});
    ^

Edit: I should also mention that if the second readAllAlloc is removed, no memory leak is detected.

@LemonBoy
Copy link
Contributor

LemonBoy commented Mar 6, 2020

CC @daurnimator

This is caused by the use of a Buffer in readAllBuffer, the second call reads exactly zero bytes since the first readAllAlloc moved the seek cursor to the end and the temporary Buffer is resized to 0 + 1 bytes. This is not ok as the presence of the trailing zero is not transferred to the callee as the return is just []u8 and the underlying 1-sized slice is never freed.

Edit: There's already an open PR (#4405)

@andrewrk andrewrk added bug Observed behavior contradicts documented or intended behavior standard library This issue involves writing Zig code for the standard library. labels Mar 6, 2020
@andrewrk andrewrk added this to the 0.6.0 milestone Mar 6, 2020
leroycep added a commit to leroycep/zig that referenced this issue Mar 7, 2020
Now that the memory leak mentioned in ziglang#4656 has been fixed.
leroycep added a commit to leroycep/zig that referenced this issue Apr 11, 2020
Now that the memory leak mentioned in ziglang#4656 has been fixed.
leroycep added a commit to leroycep/zig that referenced this issue Apr 16, 2020
Now that the memory leak mentioned in ziglang#4656 has been fixed.
leroycep added a commit to leroycep/zig that referenced this issue May 1, 2020
Now that the memory leak mentioned in ziglang#4656 has been fixed.
andrewrk pushed a commit that referenced this issue May 26, 2020
Now that the memory leak mentioned in #4656 has been fixed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Observed behavior contradicts documented or intended behavior standard library This issue involves writing Zig code for the standard library.
Projects
None yet
Development

No branches or pull requests

3 participants