sync/once: panicking functions still marked as complete

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 #8118.

LGTM=dvyukov
R=golang-codereviews, dvyukov
CC=golang-codereviews
https://golang.org/cl/137350043

diff --git a/src/sync/once.go b/src/sync/once.go
index 161ae3b3e968530524c4c519382e297a684c7426..10b42fddc2f2e2ab50cec49176f6f6a1f1002278 100644 (file)
--- a/src/sync/once.go
+++ b/src/sync/once.go
@@ -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
@@ -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
@@ -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)
        }
 }

diff --git a/src/sync/once_test.go b/src/sync/once_test.go
index 8afda82f3e18dc74f77209c4d652eaa9a49c4d48..10beefde35d512f5aee39493ec84cbfcebf815f2 100644 (file)
--- a/src/sync/once_test.go
+++ b/src/sync/once_test.go
@@ -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() {