Puzzle: make a sync.Once fire more than once

[Carl Mastrangelo]

Hi gophers,

I was playing around with a puzzle trying to break the sync package and found something cool.   Can you think of a definition for f that causes once.Do to execute the argument more than once?


package main

import (
    "sync"
)

func main() {
    var once sync.Once
    var f = // ...

    once.Do(f)
}


HIGHLIGHT BELOW FOR ANSWER


package main

import (
    "fmt"
    "sync"
)

func main() {
    var once sync.Once
    var f func()
    times := 9
    f = func() {
        if times == 0 {
            return
        }
        times--
        fmt.Println("Called")
        oldonce := once
        *&once = sync.Once{}
        once.Do(f)
        once = oldonce

    }
    once.Do(f)
}

HIGHLIGHT ABOVE FOR ANSWER

[Chris G]

I've been tempted to reset the sync.Once inside the function, but I believe there's a subtle bug -- I'd be very interested to know if my understanding is incorrect.

When Do() is called for the first time, it can aquire a mutex, and defers the unlocking until after the supplied function has completed.  If you reset the state of the Once inside the supplied function, the mutex state gets reset as well -- meaning that the defer would unlock a mutex that hadn't ever been locked.

Calling the Do() again with in the supplied function is...asking for more problems, and starts getting a bit harder to reason about than I care to try on a sunday evening.

It'd be much better to use something like https://godoc.org/golang.org/x/sync/singleflight  to deduplicate calls -- or have a secondary mutex that gets aquired to occasionally reset the once.

[Daniel Skinner]

I'd note the following:

https://play.golang.org/p/U_lmt-go6M

Also, this can be simplified to the following:

https://play.golang.org/p/eFnM98yRLk

Such a puzzle should look something more like:

https://play.golang.org/p/8KD6INL9QP

--
You received this message because you are subscribed to the Google Groups "golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[roger peppe]

This trick is used in the standard library (see net.http.envOnce.reset), and for
testing purposes it's just about OK (though still dubious IMHO) but you should
never use it in production code, as you're writing atomic state with non-atomic
operations.

On 20 November 2017 at 00:24, Carl Mastrangelo

[Marvin Renich]

* Carl Mastrangelo <carl.mas...@gmail.com> [171119 19:25]:

The call to «once.Do(f)» inside f is on an explicitly new instance of
Once that is completely independent of the original, as well as
independent of the other instances of Once in the recursive calls to f.

I do not understand why you believe that this shows once.Do invoking its
argument more than once.

The implementation of Once uses a uint32 and a sync.Mutex. A Mutex is
not safe to copy after it has been used (this is explicitly stated in
the Mutex documentation), thus Once is not safe to copy after it has
been used. This was clear to me, but perhaps this should be added to
the sync.Once documentation.

You are creating new instances of Once which are copies of other
instances both at «oldonce := once» and «once = oldonce». This makes
calling once.Do(f) more than once outside of f a violation of the
sync.Mutex contract. The correct way to use Once is to pass around a
pointer to an instance, which ensures all invocations of Do use the same
underlying uint32 and Mutex.

I believe, though I haven't proved it to myself, that if multiple go
routines use the same instance of Once, but one of the go routines makes
a copy of that instance, the copy could start out with its Mutex in a
locked state with nothing to unlock it.

...Marvin

[Axel Wagner]

Note, that go vet catches this mistake:

axelw@axelw-1 /tmp$ cat foo.go

package main

import (

"fmt"

"sync"

)

func main() {

var once sync.Once

var f func()

times := 9

f = func() {

if times == 0 {

return

}

times--

fmt.Println("Called")

oldonce := once

*&once = sync.Once{}

once.Do(f)

once = oldonce

}

once.Do(f)

}

axelw@axelw-1 /tmp$ go vet foo.go

foo.go:18: assignment copies lock value to oldonce: sync.Once contains sync.Mutex

foo.go:21: assignment copies lock value to once: sync.Once contains sync.Mutex

--
You received this message because you are subscribed to the Google Groups "golang-nuts" group.

To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts+unsubscribe@googlegroups.com.

[Marvin Renich]

* roger peppe <rogp...@gmail.com> [171120 09:35]:

> This trick is used in the standard library (see net.http.envOnce.reset), and for
> testing purposes it's just about OK (though still dubious IMHO) but you should
> never use it in production code, as you're writing atomic state with non-atomic
> operations.

When I read this, I was afraid something might be wrong in the net.http
code. But by looking at the code, it is clear that what you meant by
"this trick" was replacing an existing instance of sync.Once with a new
instance, as opposed to copying an already-used instance of sync.Once
and using both the copy and the original.

The latter is completely broken. The former might be safe as long as
the assignment to the variable containing the Once is properly
synchronized with any other assignments or reads. It is not clear from
the code in transport.go whether any synchronization is necessary; there
is none in envOnce.

...Marvin