From b22dc6385d66ac2e74afb9a5d503394fc7273d81 Mon Sep 17 00:00:00 2001 From: Rob Pike Date: Tue, 16 Sep 2014 14:00:01 -0700 Subject: [PATCH] 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 --- src/sync/once.go | 7 +++++-- src/sync/once_test.go | 8 ++++++-- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/src/sync/once.go b/src/sync/once.go index 161ae3b3e96..10b42fddc2f 100644 --- 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 8afda82f3e1..10beefde35d 100644 --- 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() {