vlang

/

v Public
0 commits 39 issues 0 pull requests 0 contributors Discussions Projects CI

sync.Once: do/do_with_param can return before the callback completes (done flag set before f()) #8

Description

sync.Once.do / do_with_param set the "done" flag before running the callback, so a concurrent caller can observe the Once as done and return while the callback has not finished (or not started). This breaks the fundamental Once guarantee that when any do() call returns, the side effects of f() are complete and visible.

Code

vlib/sync/once.v:

pub fn (mut o Once) do(f fn ()) {
    if stdatomic.load_u64(&o.count) < 1 {
        o.do_slow(f)
    }
}

fn (mut o Once) do_slow(f fn ()) {
    o.m.lock()
    if o.count < 1 {
        stdatomic.store_u64(&o.count, 1) // <-- done flag set BEFORE f()
        f()
    }
    o.m.unlock()
}

(do_with_param / do_slow_with_param have the identical shape.)

Race

  • Thread A: load count == 0do_slowlockstore count = 1 → starts running f().
  • Thread B: load count == 1 (A set it, but f() has not completed) → the fast-path if ... < 1 is false → B skips do_slow entirely and returns immediately, before f()'s effects are visible.So a second caller can proceed as if initialization is done when it is not. For lazy singletons this is a nil/uninitialized-pointer dereference on concurrent first use.

Expected

After any do()/do_with_param() returns, f() must have completed. Go's sync.Once sets done = 1 after f() (via defer) precisely for this reason.

Suggested fix

Store the done flag only after the callback runs, e.g.:

fn (mut o Once) do_slow(f fn ()) {
    o.m.lock()
    if o.count < 1 {
        f()
        stdatomic.store_u64(&o.count, 1)
    }
    o.m.unlock()
}

The fast-path load < 1 then correctly gates on completion. (A caller that loses the race still blocks on o.m.lock() in do_slow and re-checks count, so f() still runs exactly once.) The same change applies to do_slow_with_param.

Found via

Surfaced while reviewing a lazy process-global initializer (a Codex review on vlang/v#27412); worked around there by initializing the global eagerly in _vinit() instead of via Once. Filing so the underlying Once ordering can be fixed for everyone.

[!NOTE] You can use the 👍 reaction to increase the issue's priority for developers.

Please note that only the 👍 reaction to the issue itself counts as a vote. Other reactions and those to comments will not be taken into account.