FWIW, here's the pattern I've settled on for my actual application. https://play.golang.org/p/OBVsp-Rjknf
It uses sync.Mutex only and includes the mutex as a member of the guardedState struct. The deciding factor was that my industrial control application has a very large state struct with over 500 members. Using a mutex instead of atomic.Value permits me to pass the address of the g.v directly to the update function. // directUpdate is an order of magnitude faster than update for very large // structs because it avoids 2 struct copies. Use it if the update function, f, // can be made error-proof. func (g *guardedState) directUpdate(f func(*state)) { g.mu.Lock() defer g.mu.Unlock() f(&g.st) } For cases where f cannot be made error-proof, I've kept the copy/modify/store pattern of the original update method and added some error-handling, thus: // update applies an update function, f, to a copy of the state struct, and // saves the updated copy back to g.v unless f returns error. If f returns an // error, leaves g.v unchanged. func (g *guardedState) update(f func(*state) error) (err error) { g.mu.Lock() defer g.mu.Unlock() s := g.st err = f(&s) if err != nil { return } g.st = s return } With a 500 member struct, I got the following comparative benchmarks: BenchmarkUpdate-8 2000000 964 ns/op BenchmarkDirectUpdate-8 20000000 60.2 ns/op I appreciate the helpful discussions. Thank you, Skip, Matt, and Manilo. On Sunday, April 14, 2019 at 7:47:41 PM UTC-4, Skip wrote: > > I believe you only need one or the other; I assumed a use case like this: > > https://play.golang.org/p/nSgyiXU87XN > > > On Sun, Apr 14, 2019 at 1:33 PM <michae...@gmail.com <javascript:>> wrote: > >> I found a good discussion in the FAQ that explains the problem clearly. >> https://golang.org/doc/faq#methods_on_values_or_pointers >> >> I think the mutex is needed on the update function with or without the >> use of sync/atomic. The atomic guards the Load and the Store but the func >> is operating on the copy returned by Load. Seems to me it's vulnerable to >> being pre-empted unless the entire read/modify/write sequence is guarded by >> a mutex. >> >> On Sunday, April 14, 2019 at 2:20:02 PM UTC-4, Skip wrote: >>> >>> I don't know if it's documented or not. In the language reference you >>> can see the rules for method calls: >>> >>> https://golang.org/ref/spec#Calls >>> https://golang.org/ref/spec#Method_sets >>> >>> A hint might have been that object should have mutated, but it didn't. >>> It's in a class of errors that becomes recognizable once you've been bitten >>> by it. >>> >>> In the example you gave, use of mutex seems redundant to me; execution >>> of func passed into update is already guarded by atomic. >>> >>> On Sun, Apr 14, 2019 at 9:51 AM <michae...@gmail.com> wrote: >>> >>>> Thanks, Skip. That fixes it. Is the need for a pointer receiver >>>> documented somewhere? It's not something that even crossed my mind given >>>> that the neither the compiler nor golint complained. I suppose it makes >>>> sense if I think of func (x) foo(y) {} as being an alternate way of >>>> writing >>>> func foo(x, y) {}. In that case, it's clear that a copy of x is being >>>> passed since that's Go's default. >>>> >>>> While this topic is still alive, I'd like to ask a follow-on question: >>>> Is the use of sync/atomic actually needed in this example or is it >>>> sufficient to wrap all accesses in mutex Lock/Unlock (using the same >>>> mutex, >>>> of course). >>>> >>>> >>>> >>>> On Sunday, April 14, 2019 at 12:08:55 PM UTC-4, Skip wrote: >>>>> >>>>> The receiver for load and update should be the original object not a >>>>> copy. >>>>> >>>>> https://play.golang.org/p/XCZC0OVhGMa >>>>> >>>>> On Sun, Apr 14, 2019, 7:56 AM <michae...@gmail.com> wrote: >>>>> >>>>>> >>>>>> https://play.golang.org/p/6aQYNjojyBD >>>>>> >>>>>> I'm clearly missing something about the way sync.Mutex and >>>>>> atomic.Value work in Go. >>>>>> >>>>>> I'm attempting to write a pair of concurrency safe methods, load() >>>>>> and update(), for accessing a struct. The struct is stored as an >>>>>> atomic.Value and accessed with atomic.Load and atomic.Value. I'm also >>>>>> wrapping the accesses within a mutex Lock/Unlock. That's probably >>>>>> unneeded >>>>>> for my load() method but I added it trying to figure out why it's not >>>>>> returning updated info. In my minimal example below (also in the Go >>>>>> Playground link above) the output of main() should be: >>>>>> >>>>>> s={65535} >>>>>> v={65535} >>>>>> >>>>>> but I get >>>>>> >>>>>> s={65535} >>>>>> v={0} >>>>>> >>>>>> indicating that the updated value is not available after the call to >>>>>> update(). >>>>>> >>>>>> The only thing I'm doing that's a little different from the examples >>>>>> in the doc for sync/atomic is passing a function that takes a pointer to >>>>>> a >>>>>> struct instance to my update function. I do that to make it easy to >>>>>> write >>>>>> code that updates just a few items in the state struct (which in my real >>>>>> application has many members instead of just one as shown here.) >>>>>> >>>>>> Apologies for wasting the group's time if I've overlooked a >>>>>> brain-dead error, but I've been fooling with this for several hours now >>>>>> and >>>>>> can't see why it shouldn't be working, >>>>>> >>>>>> >>>>>> package main >>>>>> >>>>>> import ( >>>>>> "fmt" >>>>>> "sync" >>>>>> "sync/atomic" >>>>>> ) >>>>>> >>>>>> type state struct { >>>>>> R4000 uint16 >>>>>> } >>>>>> type guardedState struct { >>>>>> v atomic.Value >>>>>> } >>>>>> >>>>>> var stateMutex = sync.Mutex{} >>>>>> var gState guardedState >>>>>> >>>>>> func init() { >>>>>> gState.v.Store(state{}) >>>>>> } >>>>>> >>>>>> func (g guardedState) load() state { >>>>>> stateMutex.Lock() >>>>>> defer stateMutex.Unlock() >>>>>> s := gState.v.Load() >>>>>> return s.(state) >>>>>> } >>>>>> >>>>>> func (g guardedState) update(f func(*state)) { >>>>>> stateMutex.Lock() >>>>>> defer stateMutex.Unlock() >>>>>> s := g.v.Load().(state) >>>>>> f(&s) >>>>>> g.v.Store(s) >>>>>> } >>>>>> >>>>>> func main() { >>>>>> f := func(s *state) { >>>>>> s.R4000 = 65535 >>>>>> fmt.Printf("s=%v\n", *s) >>>>>> } >>>>>> gState.update(f) >>>>>> v := gState.load() >>>>>> fmt.Printf("v=%v\n", v) >>>>>> } >>>>>> >>>>>> -- >>>>>> 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 golan...@googlegroups.com. >>>>>> For more options, visit https://groups.google.com/d/optout. >>>>>> >>>>> -- >>>> 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 golan...@googlegroups.com. >>>> For more options, visit https://groups.google.com/d/optout. >>>> >>> -- >> 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 golan...@googlegroups.com <javascript:>. >> For more options, visit https://groups.google.com/d/optout. >> > -- 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+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.