vlang / v

Simple, fast, safe, compiled language for developing maintainable software. Compiles itself in <1s with zero library dependencies. Supports automatic C => V translation. https://vlang.io
MIT License
35.8k stars 2.17k forks source link

defer not working correctly from loops #14701

Open emdete opened 2 years ago

emdete commented 2 years ago

OS: linux, Devuan GNU/Linux 5 (daedalus/ceres) Processor: 8 cpus, 64bit, little endian, Intel(R) Core(TM) i7-8550U CPU @ 1.80GHz CC version: cc (Debian 11.3.0-3) 11.3.0

What did you do?

fn f(k int){
    eprintln(k)
}

fn main() {
    mut i := 0
    for i<10 {
        i++
        defer{f(i)}
    }
}

What did you expect to see?

output of 10 lines

What did you see instead?

only one line, the defer is executed once only.

this is the similar code in go:

package main
import "log"

func main() {
    i := 0
    for i<10{
        defer log.Printf("close %d", i)
        i += 1
    }
    log.Printf("done")
}

which works as expected.

jrfondren commented 2 years ago

"output of 10 lines" is obtained from more than one way to implement defer.

Consider:

# nim
proc main =
  for n in 1..3:
    defer: echo n
  echo "done"
main()
// d
void main() {
    import std.stdio;
    foreach (n; 1..4)
        scope(exit) writeln(n);
    writeln("done");
}
// zig
const print = @import("std").debug.print;

pub fn main() void {
    for ([_]i32{ 1, 2, 3 }) |n| {
        defer print("{}\n", .{n});
    }
    print("done\n", .{});
}

These all produce the following output:

1
2
3
done

These languages have this output as the deferred statement runs when the current scope is exited, which happens on each iteration of the loop, rather than when the function exits. Go, for completeness:

package main

import "fmt"

func main() {
    for i := 1; i <= 3; i++ {
        defer fmt.Println(i)
    }
    fmt.Println("done")
}

has this output:

done
3
2
1

Because there's this divergence in what defer does, and because v's docs don't use defer in a loop, I wasn't sure that v's version was buggy or an innovation:

fn main() {
    for i in [1, 2, 3] {
        defer { println(i) }
    }
    println('done')
}

output:

done
1

Reasons for it to be an innovation: it's very efficient to rewrite functions so that all the defer blocks happen on exit, rather than pushing closures to a stack like go. Reasons for it to be a bug: natural and less-trivial uses of defer won't work, like opening files in a loop while defering the close:

import os

fn main() {
    for name in ['file1', 'file2', 'file3'] {
        mut f := os.create(name) or { panic(err) }
        defer {
            println('closing ' + name)
            f.close()
        }

        // do some work
        f.writeln('just opened ' + name) or {}
    }
}

(this is an accidental case for deferring to the end of the loop like nim/d/zig. go wouldn't close any of these files until the end of the function, putting it at risk of hitting open file limits)

But having said all that, here's a big reason to think that the current v behavior is indeed buggy:

$ v run vfile.v
V panic: malloc_noscan(-89406639 <= 0)
v hash: 095f4bc
/tmp/v_1000/vfile.17441036274052130846.tmp.c:7411: at _v_panic: Backtrace
/tmp/v_1000/vfile.17441036274052130846.tmp.c:7647: by malloc_noscan
/tmp/v_1000/vfile.17441036274052130846.tmp.c:10740: by string__plus
/tmp/v_1000/vfile.17441036274052130846.tmp.c:17083: by main__main
/tmp/v_1000/vfile.17441036274052130846.tmp.c:17466: by main

that last example segfaults. If you remove the println() from the defer, it no longer segfaults, but it's still undesirable. Here's a working example:

import os

fn writes() {
    for name in ['file1', 'file2', 'file3'] {
        mut f := os.create(name) or { panic(err) }
        defer { f.close() }
        f.writeln('just opened ' + name) or {}
    }
}

fn main() {
    writes()
    for true {
        // do nothing
    }
}

Run that, and then look at its working directory:

-rw-r--r--  1 julian julian   18 Aug  5 05:16 file3
-rw-r--r--  1 julian julian    0 Aug  5 05:16 file2
-rw-r--r--  1 julian julian    0 Aug  5 05:16 file1

Only file3 was properly closed, causing the write to be flushed. file2 and file1 are empty files that, weren't leaked (?), but were closed inappropriately when writes() returned to main(), with the result that the write to them was lost.