proposal: sync: add Done() bool method to sync.Once

[niktri]

What version of Go are you using (go version)?

$ go version
go version go1.15 darwin/amd6

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go env Output

$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/niktri/Library/Caches/go-build"
GOENV="/Users/niktri/Library/Application Support/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/niktri/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/niktri/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/darwin_amd64"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/niktri/nik/dev/me/go/p1/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/85/by_j74dj2jg9yx873rkc0_nw0000gn/T/go-build331637869=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

sync.Once.Do() is nice utility for doing one-time initialization or clean-up.
I often miss Once.Done(), which will be useful for

Usecase 1: A separate go-routine or a web service is continuously polling the status of work.
Usecase 2: All methods first checks initialization, then does real work. Below is the current way to handle this.

type AnimalStore struct {once   sync.Once;inited uint32}
func (a *AnimalStore) Init() {// May be called concurrently as other methods
	a.once.Do(func() {
		longOperationSetupDbOpenFilesQueuesEtc()
		atomic.StoreUint32(&a.inited, 1)
	})
}
func (a *AnimalStore) CountOfCats() (int, error) { // Called in another go-routine
	if atomic.LoadUint32(&a.inited) == 0 { // All such methods first check inited flag then do real operation.
		return 0, NotYetInitedError
	}
        //Real operation
}

Since sync.Once already checks a done flag before calling Do(), it is natural for it to expose it.
I propose to add below Done() method to sync.Once struct.

//Done is Lock-free way to check if Once.Do() is completed or not.
func (o *Once) Done() bool {return atomic.LoadUint32(&o.done) == 1}

This would help avoid clients duplicating it with potentially bug-prone atomic operations.

What did you expect to see?

What did you see instead?

[robpike]

No need for this. Just call once.Do as many times as you want. When it returns, the operation is done and you can proceed.

[andybons]

@niktri does @robpike’s solution satisfy your needs?

[niktri]

@andybons @robpike once.Do() will 1) Actually Do the work. 2) Will block.
Currently there is no non-blocking way to just poll the status without Doing work.

Proposed method will return status without blocking or performing real work.

func (o *Once) Done() bool {return atomic.LoadUint32(&o.done) == 1}

[DeedleFake]

Usecase 1: sync.Once is not supposed to be for communication. It's for thread-safe lazy initialization, and similar situations. If you need to check if something's been initialized or not to, for example, give status to the user, that's not really the domain of sync.Once. Use a channel or some other communication-oriented system, or even a separate variable that you manually use atomic with yourself, as you showed.

Usecase 2: Why do you want to do this? Optimization? If you just want something to initialize, initialize it. Just call once.Do() at the beginning of every code path that's going to need it. If you need to check initialization status concurrently, see what I said above.

[niktri]

@DeedleFake status checker routine may not want to Do actual work, if it's not already started. It just wants to check the status.
Besides it may not have sufficient data to Do work, e.g. init config.

The reason why someone is using sync.Once & not directly mutex + atomic is: It's simpler.
If client code has to keep separate done flag, she has to be careful not to read it unsafe without using atomic.

I feel that if sync.Once can Do something, it's natural for it to provide status of work too, in lock-free way.

[robpike]

I don't understand the point of this request. There are three things that might happen.

1. You ask, and discover the operation has run. You might as well have called once.Do.

2. You ask, and discover the operation has not run, and you know no part of the program is free to run it. That means you own the flow of control and know a priori that the operation cannot run, and there was no point in asking. You will run once.Do when the time is right.

3. You ask, and discover the operation has not run, but some part of the program is free to run it at that time. The answer is racy, and by the time you act on it the operation might have run. This is an unsafe scenario and you should avoid it, as any calculation you might perform knowing that the operation has not run might have its invariant violated. It is not a safe way to program atomic operations.

[ianlancetaylor]

sync.Once does one job. You are asking it to do a different job. You need a different construct, which you can write yourself. If you like, you can start with the implementation of sync.Once; it's only 69 lines, which are mostly comments.

I'm going to close this issue.

[niktri]

Thanks everyone for your time. I guess it's a special usecase & pretty easy to implement own custom sync.Once.

Just did it.