Here is a slightly easier version to see the race between the mutation and the
copy for the value method:
package main
import (
"log"
"sync"
)
type S struct {
lock *sync.Mutex
index int
values [128]int
}
func (s *S) mutate() {
s.lock.Lock();
defer s.lock.Unlock();
s.index++;
for i:=0; i< 128; i++ {
s.values[i]=s.index;
}
}
func (s S) validate() {
for i:=0;i<128;i++ {
if s.values[i]!=s.index {
log.Fatal("mismatch error")
}
}
}
func doit(s *S) {
for {
s.mutate()
s.validate()
}
}
func main() {
var s S
var lock sync.Mutex
s.lock = &lock
var wg sync.WaitGroup
wg.Add(1)
for i:=0;i<64;i++ {
go doit(&s)
}
wg.Wait()
}
> On Oct 7, 2024, at 6:06 PM, robert engels <[email protected]> wrote:
>
> I wrote a simple test. Sure enough it fails, and it reports a data race.
>
> package main
>
> import (
> "log"
> "sync"
> )
>
> type S struct {
> sync.Mutex
> index int
> values [128]int
> }
>
> func (s *S) mutate() {
> s.Lock();
> defer s.Unlock();
> s.index++;
> for i:=0; i< 128; i++ {
> s.values[i]=s.index;
> }
> }
>
> func (s S) validate() {
> for i:=0;i<128;i++ {
> if s.values[i]!=s.index {
> log.Fatal("mismatch error")
> }
> }
> }
>
> func doit(s *S) {
> for {
> s.mutate()
> s.validate()
> }
> }
>
> func main() {
> var s S
> var wg sync.WaitGroup
> wg.Add(1)
> for i:=0;i<64;i++ {
> go doit(&s)
> }
> wg.Wait()
> }
>
> In fact, you get a linter warning, because of the copy of the mutex in
> calling the value method - since it knows it should be a reference.
>
>
>> On Oct 7, 2024, at 5:30 PM, Robert Engels <[email protected]
>> <mailto:[email protected]>> wrote:
>>
>> I am fairly certain if you mix pointer and receiver methods and the receiver
>> methods mutate - even if you synchronize those you will get a data race
>> calling the value methods. It must afaik as the runtime/compiler has no
>> implicit synchronization when creating the copies. That is a data race.
>>
>>> On Oct 7, 2024, at 5:10 PM, Axel Wagner <[email protected]
>>> <mailto:[email protected]>> wrote:
>>>
>>>
>>> My argument had nothing to do with synchronization.
>>>
>>> FTR I find the synchronization argument also extremely dubious. By that
>>> argument, you also can't pass the address to a local variable to another
>>> function, when using it as a value elsewhere. It's a weird argument to
>>> make. time.Time uses a mix of pointer- and value receivers and IMO no one
>>> can make a serious argument that this would expose programs to risks of
>>> data races.
>>>
>>> But to repeat my actual argument in favour of (sometimes) mixing receiver
>>> kinds:
>>> 1. It is totally reasonable to use some types as values.
>>> 2. Such types, intended to be used as values, will need to use
>>> value-receivers for some methods, as otherwise their value-version does not
>>> implement certain interfaces (methods are not promoted from pointer to
>>> value types). Like fmt.Stringer, for example. And
>>> 3. such types still need to sometimes use pointer-receivers, to implement
>>> functionalities like unmarshalling.
>>>
>>> time.Time is a standard library example of such a type. I also provided an
>>> example for an "enum-like" type implementing flag.Value.
>>>
>>> On Mon, 7 Oct 2024 at 23:57, Robert Engels <[email protected]
>>> <mailto:[email protected]>> wrote:
>>> I am pretty sure it is immaterial. If the object isn’t immutable any copy
>>> or mutation operation needs to be synchronized.
>>>
>>> But the problem afaik is that you can’t control synchronization when the
>>> object is copied for a value receiver - which means you cant properly
>>> synchronize when you have pointer and value receivers unless you do it
>>> externally (which is a huge pain to do everywhere).
>>>
>>>> On Oct 7, 2024, at 4:43 PM, 'Axel Wagner' via golang-nuts
>>>> <[email protected] <mailto:[email protected]>> wrote:
>>>>
>>>>
>>>> No offence, but I made an argument. You don't have to agree with the
>>>> argument and it might be wrong. But to convince me, at least, that
>>>> argument would need to actually be referenced.
>>>>
>>>> I gave reasons why, in my opinion, *not* mixing value and pointer
>>>> receivers sometimes leads to incorrect code. So as far as I'm concerned
>>>> (until someone tells me my reasons are wrong) Goland's linter simply
>>>> encourages you to write bad code. It would not be the first time that I
>>>> strongly disagree with the recommendations of an IDE. Goland in particular
>>>> has a history of making, in my opinion, pretty questionable decisions.
>>>>
>>>> On Mon, 7 Oct 2024 at 22:39, Cleberson Pedreira Pauluci
>>>> <[email protected] <mailto:[email protected]>> wrote:
>>>> Many places and books I've read generally say: If a function needs to
>>>> update a variable, or if an argument is so large that we want to avoid
>>>> copying it, we should pass the pointer. Same for methods (pointer
>>>> receiver). (The Go programming language book).
>>>>
>>>> About mixing "value receiver" and "pointer receiver". Even the IDE
>>>> complains about this and recommends following the Go documentation.
>>>> (Goland)
>>>>
>>>>
>>>> Em segunda-feira, 7 de outubro de 2024 às 15:15:25 UTC-3, burak serdar
>>>> escreveu:
>>>> Mixing pointer and value receivers can be race-prone, because of the
>>>> copying involved in passing value receivers.
>>>>
>>>> On Mon, Oct 7, 2024 at 12:03 PM 'Axel Wagner' via golang-nuts
>>>> <[email protected] <>> wrote:
>>>> >
>>>> > To be honest, I always found this recommendation a little bit strange,
>>>> > personally.
>>>> >
>>>> > I'll note that the standard library does not really keep to this either.
>>>> > For example, time.Time.UnmarshalText (obviously) has a pointer-receiver,
>>>> > while almost all other methods on time.Time have a value receiver.
>>>> > And if you implement flag.Value, the Set method obviously needs a
>>>> > pointer receiver, but if the String method has one as well, it won't
>>>> > print properly when used as a value. In basically every implementation
>>>> > of flag.Value I've ever written, String needed a value receiver, while
>>>> > Set needed a pointer receiver.
>>>> >
>>>> > I understand the basic idea of the advice, that if a type keeps state
>>>> > that is manipulated via methods, then it should generally be passed
>>>> > around as a pointer, so giving all the methods a pointer-receiver works
>>>> > well. But if a type *is* intended to be used as a value (like time.Time
>>>> > or Enum in my example) then you will almost certainly end up with a mix
>>>> > of receiver kinds - as soon as you want to add any form of
>>>> > de-serialization to it. So "don't mix receiver kinds" seems like
>>>> > misleading advice to me.
>>>> >
>>>> > On Mon, 7 Oct 2024 at 19:44, Ian Lance Taylor <[email protected] <>>
>>>> > wrote:
>>>> >>
>>>> >> On Mon, Oct 7, 2024 at 10:29 AM Ken Lee <[email protected] <>>
>>>> >> wrote:
>>>> >> >
>>>> >> > ---
>>>> >> > There is a consideration to make, though: historically it has been
>>>> >> > considered bad form in Go to give a type a mix of value and pointer
>>>> >> > receivers in methods without a very specific reason for doing so.
>>>> >> > ---
>>>> >> >
>>>> >> > Is this still the case now? As in 2024.
>>>> >>
>>>> >> As a general guideline, yes.
>>>> >>
>>>> >> https://go.dev/wiki/CodeReviewComments#receiver-type
>>>> >> <https://go.dev/wiki/CodeReviewComments#receiver-type>
>>>> >>
>>>> >> Ian
>>>> >>
>>>> >>
>>>> >>
>>>> >> > On Sunday 13 January 2013 at 7:03:29 am UTC+8 Kevin Gillette wrote:
>>>> >> >>
>>>> >> >> Indeed. In addition to implicit dereferencing for value receivers,
>>>> >> >> the reverse also works as well: anything that is addressable
>>>> >> >> (including 'value' variables on the stack, or a field of element of
>>>> >> >> anything that's addressable) will implicitly be addressed when a
>>>> >> >> pointer-receiver method is called on them (though you must
>>>> >> >> explicitly use the address operator when you need to pass value
>>>> >> >> variables as pointers).
>>>> >> >>
>>>> >> >> There is a consideration to make, though: historically it has been
>>>> >> >> considered bad form in Go to give a type a mix of value and pointer
>>>> >> >> receivers in methods without a very specific reason for doing so.
>>>> >> >> The typical justification is that a small struct in a getter method
>>>> >> >> might as well have a value receiver even though the corresponding
>>>> >> >> setter method uses a pointer receiver; this, however, can lead to
>>>> >> >> confusion on the part of the app programmer if they start out using
>>>> >> >> only the read-only methods upon what turns out to be a value-copy of
>>>> >> >> the original (but hey, it compiled and seems to work, so it must be
>>>> >> >> correct) -- when use of pointer-receiver methods don't seem to
>>>> >> >> produce the documented changes in the original, it can be difficult
>>>> >> >> to debug.
>>>> >> >>
>>>> >> >>
>>>> >> >> On Saturday, January 12, 2013 3:17:16 PM UTC-7, Dave Collins wrote:
>>>> >> >>>
>>>> >> >>> On Saturday, January 12, 2013 3:52:35 PM UTC-6, Taric Mirza wrote:
>>>> >> >>>>
>>>> >> >>>> Thanks! Works like a charm and is helping cleaning up my code a
>>>> >> >>>> ton.
>>>> >> >>>>
>>>> >> >>>> One other question, this is really more about coding style:
>>>> >> >>>>
>>>> >> >>>> In the case where you manipulate members of the struct, then using
>>>> >> >>>> pointers as in your example is the way to go.
>>>> >> >>>>
>>>> >> >>>> But, you have a choice for functions that just read values from
>>>> >> >>>> the
>>>> >> >>>> struct instead of manipulating it. Is there a best practice coding
>>>> >> >>>> style here, between dereferencing the struct and then using that,
>>>> >> >>>> or
>>>> >> >>>> dereferencing each member of the struct as you go? eg:
>>>> >> >>>>
>>>> >> >>>> // A:
>>>> >> >>>>
>>>> >> >>>> laser := worldobj.(*Laser)
>>>> >> >>>> fmt.Printf("%0.4f,%0.4f", (*laser).x, (*laser).y)
>>>> >> >>>>
>>>> >> >>>> versus
>>>> >> >>>>
>>>> >> >>>> // B:
>>>> >> >>>>
>>>> >> >>>> laser := *(worldobj.(*Laser))
>>>> >> >>>> fmt.Printf("%0.4f,%0.4f", laser.x, laser.y)
>>>> >> >>>>
>>>> >> >>>>
>>>> >> >>>> I'm kind of torn. I would imagine A) has slightly better
>>>> >> >>>> performance, and doesn't require any code-rework if you later on
>>>> >> >>>> need
>>>> >> >>>> to manipulate the struct.
>>>> >> >>>>
>>>> >> >>>> On the other hand, B) is more readable since you don't have to
>>>> >> >>>> look at
>>>> >> >>>> pointers all over the place, just on one line.
>>>> >> >>>
>>>> >> >>>
>>>> >> >>> Actually, you don't need to dereference at all. Go automatically
>>>> >> >>> handles this for you.
>>>> >> >>>
>>>> >> >>> See this example: http://play.golang.org/p/ANaKaFSQLn
>>>> >> >>> <http://play.golang.org/p/ANaKaFSQLn>
>>>> >> >>>
>>>> >> > --
>>>> >> > 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 [email protected] <>.
>>>> >> > To view this discussion on the web visit
>>>> >> > https://groups.google.com/d/msgid/golang-nuts/03df7dce-5c48-44a3-bc3c-851ded2a1f08n%40googlegroups.com
>>>> >> >
>>>> >> > <https://groups.google.com/d/msgid/golang-nuts/03df7dce-5c48-44a3-bc3c-851ded2a1f08n%40googlegroups.com>.
>>>> >> >
>>>> >>
>>>> >> --
>>>> >> 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 [email protected] <>.
>>>> >> To view this discussion on the web visit
>>>> >> https://groups.google.com/d/msgid/golang-nuts/CAOyqgcX7v9Edk5beRH38tfJO18ZUXv-nOHsEPPCfMQy0hz%3DFdw%40mail.gmail.com
>>>> >>
>>>> >> <https://groups.google.com/d/msgid/golang-nuts/CAOyqgcX7v9Edk5beRH38tfJO18ZUXv-nOHsEPPCfMQy0hz%3DFdw%40mail.gmail.com>.
>>>> >>
>>>> >
>>>> > --
>>>> > 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 [email protected] <>.
>>>> > To view this discussion on the web visit
>>>> > https://groups.google.com/d/msgid/golang-nuts/CAEkBMfGcq2nxaik_qAWoX81W-tTKRRYBDM5_6%3DefSv4tr8b03g%40mail.gmail.com
>>>> >
>>>> > <https://groups.google.com/d/msgid/golang-nuts/CAEkBMfGcq2nxaik_qAWoX81W-tTKRRYBDM5_6%3DefSv4tr8b03g%40mail.gmail.com>.
>>>> >
>>>>
>>>> --
>>>> 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 [email protected]
>>>> <mailto:[email protected]>.
>>>> To view this discussion on the web visit
>>>> https://groups.google.com/d/msgid/golang-nuts/9b28006b-c310-417e-9afc-e7f5c470641cn%40googlegroups.com
>>>>
>>>> <https://groups.google.com/d/msgid/golang-nuts/9b28006b-c310-417e-9afc-e7f5c470641cn%40googlegroups.com?utm_medium=email&utm_source=footer>.
>>>>
>>>> --
>>>> 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 [email protected]
>>>> <mailto:[email protected]>.
>>>> To view this discussion on the web visit
>>>> https://groups.google.com/d/msgid/golang-nuts/CAEkBMfEj%3DQACB31VMc7ami7xt9tMF00kYxFUfZpWfZ0j65GWsw%40mail.gmail.com
>>>>
>>>> <https://groups.google.com/d/msgid/golang-nuts/CAEkBMfEj%3DQACB31VMc7ami7xt9tMF00kYxFUfZpWfZ0j65GWsw%40mail.gmail.com?utm_medium=email&utm_source=footer>.
>>>
>>>
>>> --
>>> 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 [email protected]
>>> <mailto:[email protected]>.
>>> To view this discussion on the web visit
>>> https://groups.google.com/d/msgid/golang-nuts/CAEkBMfFYZ1DTD9fTVzNHtOp7Ed7w3_x8QbxsB2x_%2BTs%3DtxY0BA%40mail.gmail.com
>>>
>>> <https://groups.google.com/d/msgid/golang-nuts/CAEkBMfFYZ1DTD9fTVzNHtOp7Ed7w3_x8QbxsB2x_%2BTs%3DtxY0BA%40mail.gmail.com?utm_medium=email&utm_source=footer>.
>>
>>
>> --
>> 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 [email protected]
>> <mailto:[email protected]>.
>> To view this discussion on the web visit
>> https://groups.google.com/d/msgid/golang-nuts/B6F948A5-9F2E-4698-85D1-17B862779901%40ix.netcom.com
>>
>> <https://groups.google.com/d/msgid/golang-nuts/B6F948A5-9F2E-4698-85D1-17B862779901%40ix.netcom.com?utm_medium=email&utm_source=footer>.
>
--
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 [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/golang-nuts/ECF09796-9D2F-468B-812C-56ACCD0FA273%40ix.netcom.com.