sync::Once and panicking closure

[tmiasko]

The documentation for sync::Once does not clarify what happens if given closure
panics. Currently I would rather expect that call_once will execute the closure
only once, whether it panics or not. But this is not what happens:

#![feature(recover)]

use std::sync::*;
use std::panic::recover;

static ONCE : Once = ONCE_INIT;

fn main() {
    for i in 1..10 {
        let _ = recover(|| { 
            ONCE.call_once(|| {
                let s = if i > 1 { "s" } else { ""};
                panic!("Once::call_once called {} time{}", i, s);
            }); 
        });
    }
}

Prints:

thread '<main>' panicked at 'Once::call_once called 1 time', <anon>:13
note: Run with `RUST_BACKTRACE=1` for a backtrace.
thread '<main>' panicked at 'Once::call_once called 2 times', <anon>:13
thread '<main>' panicked at 'Once::call_once called 3 times', <anon>:13
thread '<main>' panicked at 'Once::call_once called 4 times', <anon>:13
thread '<main>' panicked at 'Once::call_once called 5 times', <anon>:13
thread '<main>' panicked at 'Once::call_once called 6 times', <anon>:13
thread '<main>' panicked at 'Once::call_once called 7 times', <anon>:13
thread '<main>' panicked at 'Once::call_once called 8 times', <anon>:13
thread '<main>' panicked at 'Once::call_once called 9 times', <anon>:13

Either way documentation needs a slight improvement. And additionally code if
it is not a desired behaviour. As a side comment I will note that this
behaviour differs between languages. In C++ for example std::call_once to
proceed must not exit via exception, in Go on the other hand exiting via panic
counts as successful execution.

[alexcrichton]

triage: I-nominated

[alexcrichton]

I don't really have an opinion on what the semantics should be in this case, but I'm curious what others think!

[sfackler]

I personally think the current behavior is fine (a common use case for Once it to initialize something, and if the closure panics it probably hasn't been initialized), but we should document it.

[bluss]

panic inside call_once is a very broken scenario.

Maybe it would only help authors of critical unsafe code if we ensured the closure could only be called once ever. The closure call could be enforced to abort on panic (it should be a "noexcept" zone).

It's an FnOnce closure, so being called twice is an unsoundness problem, too. (See below)

[bluss]

Interesting, it's not broken because of FnOnce, the actual closure value is only called once (or never).

[aturon]

cc me

[brson]

I prefer poisoning it, without any options to unpoison. Alternately, just leaving it alone.

[alexcrichton]

The libs team discussed this during triage yesterday and we didn't necessarily reach any concrete conclusion. As @brson mentioned we talked about how we may want to just poison a Once in the same way mutexes and rwlocks are poisoned, but it also seemed that if we didn't do that the behavior here was reasonable.

[durka]

This is marked as the tracking issue for the once_poison feature. Therefore, it should be reopened or a new tracking issue created.

[alexcrichton]

Ah it's probably best to open a new tracking issue, would you be interested in doing so @durka?