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

niktri · 2020-09-29T07:42:50Z

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?

The text was updated successfully, but these errors were encountered:

robpike · 2020-09-29T09:45:03Z

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 · 2020-09-29T14:19:48Z

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

niktri · 2020-09-29T17:38:05Z

@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 · 2020-09-29T18:42:33Z

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 · 2020-10-02T11:30:16Z

@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 · 2020-10-02T13:14:36Z

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

You ask, and discover the operation has run. You might as well have called once.Do.
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.
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 · 2020-10-14T16:57:29Z

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 · 2020-10-16T09:43:42Z

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

Just did it.

andybons changed the title ~~Add Done() bool method to sync.Once~~ proposal: sync: add Done() bool method to sync.Once Sep 29, 2020

gopherbot added this to the Proposal milestone Sep 29, 2020

gopherbot added the Proposal label Sep 29, 2020

andybons added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Sep 29, 2020

andybons removed the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Sep 29, 2020

ianlancetaylor closed this as completed Oct 14, 2020

golang locked and limited conversation to collaborators Oct 16, 2021

gopherbot added the FrozenDueToAge label Oct 16, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

niktri commented Sep 29, 2020

robpike commented Sep 29, 2020

andybons commented Sep 29, 2020

niktri commented Sep 29, 2020

DeedleFake commented Sep 29, 2020

niktri commented Oct 2, 2020

robpike commented Oct 2, 2020

ianlancetaylor commented Oct 14, 2020

niktri commented Oct 16, 2020

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

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

Comments

niktri commented Sep 29, 2020

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

Does this issue reproduce with the latest release?

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

What did you do?

What did you expect to see?

What did you see instead?

robpike commented Sep 29, 2020

andybons commented Sep 29, 2020

niktri commented Sep 29, 2020

DeedleFake commented Sep 29, 2020

niktri commented Oct 2, 2020

robpike commented Oct 2, 2020

ianlancetaylor commented Oct 14, 2020

niktri commented Oct 16, 2020

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

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