Skip to content

Commit

Permalink
sync/once: panicking functions still marked as complete
Browse files Browse the repository at this point in the history
This is a corner case, and one that was even tested, but this
CL changes the behavior to say that f is "complete" even if it panics.
But don't think of it that way, think of it as sync.Once runs
the function only the first time it is called, rather than
repeatedly until a run of the function completes.

Fixes golang#8118.

LGTM=dvyukov
R=golang-codereviews, dvyukov
CC=golang-codereviews
https://golang.org/cl/137350043
  • Loading branch information
robpike authored and wheatman committed Jul 20, 2018
1 parent 13b084e commit a431fd2
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 4 deletions.
7 changes: 5 additions & 2 deletions src/sync/once.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ type Once struct {
}

// Do calls the function f if and only if Do is being called for the
// first time for this instance of Once. In other words, given
// first time for this instance of Once. In other words, given
// var once Once
// if once.Do(f) is called multiple times, only the first call will invoke f,
// even if f has a different value in each invocation. A new instance of
Expand All @@ -29,6 +29,9 @@ type Once struct {
// Because no call to Do returns until the one call to f returns, if f causes
// Do to be called, it will deadlock.
//
// If f panics, Do considers it to have returned; future calls of Do return
// without calling f.
//
func (o *Once) Do(f func()) {
if atomic.LoadUint32(&o.done) == 1 {
return
Expand All @@ -37,7 +40,7 @@ func (o *Once) Do(f func()) {
o.m.Lock()
defer o.m.Unlock()
if o.done == 0 {
defer atomic.StoreUint32(&o.done, 1)
f()
atomic.StoreUint32(&o.done, 1)
}
}
8 changes: 6 additions & 2 deletions src/sync/once_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,12 @@ func TestOncePanic(t *testing.T) {
for i := 0; i < 2; i++ {
func() {
defer func() {
if recover() == nil {
t.Fatalf("Once.Do() has not panic'ed")
r := recover()
if r == nil && i == 0 {
t.Fatalf("Once.Do() has not panic'ed on first iteration")
}
if r != nil && i == 1 {
t.Fatalf("Once.Do() has panic'ed on second iteration")
}
}()
once.Do(func() {
Expand Down

0 comments on commit a431fd2

Please sign in to comment.