Or to be perhaps more precise: This view can be useful to understand the observed behaviour: A string is two words which are updated non-atomically, hence one read might observe the write of the length separately from the write of the data.
But to determine the *correctness* of the program, any concurrent read/write of a variable must be synchronised, regardless of type or size. On Mon 25. Mar 2024 at 15:53, Axel Wagner <axel.wagner...@googlemail.com> wrote: > On Mon, Mar 25, 2024 at 3:47 PM 'Brian Candler' via golang-nuts < > golang-nuts@googlegroups.com> wrote: > >> And as Axel's own reproducer shows, even having two threads reading and >> writing the same *variable* which points to a string can result in >> indeterminate behaviour, since a string is really a struct containing two >> parts (a pointer and a len). > > > I want to caution against this reasoning. The size of a variable is > irrelevant. A data race on an `int` is still a data race. > It's the concurrent modification of a variable (in the sense of the spec > <https://go.dev/ref/spec#Variables>, i.e. every field or array element is > its own variable) that is the problem - not its type. > > >> You're not mutating the string itself, but you are updating a variable >> at the same time as it's being read. >> >> In this regard, Go is no more thread-safe than C or C++, unless you make >> use of the concurrency features it provides (i.e. channels) instead of >> concurrently reading and writing the same variables. >> >> On Monday 25 March 2024 at 13:18:15 UTC Axel Wagner wrote: >> >>> TBQH the word "mutable" doesn't make a lot of sense in Go (even though >>> the spec also calls strings "immutable"). >>> Arguably, *all* values in Go are immutable. It's just that all >>> *pointers* in Go allow to modify the referenced variables - and some types >>> allow you to get a pointer to a shared variable, which strings don't. >>> >>> That is, a `[]byte` is immutable - you have to write `x = append(x, v)` >>> specifically because `append` creates a new slice value and overwrites the >>> variable `x` with it. >>> However, a `[]byte` refers to an underlying array and `&b[0]` allows you >>> to obtain a pointer to that underlying array. So a `[]byte` represents a >>> reference and that reference allows to mutate the referenced storage >>> location. The same goes for a `*T`, a `map[K]V`, or a `type S struct{ X >>> int; P *int }` - `S` itself is immutable, but `S.X` is a reference to some >>> potentially shared variable. >>> >>> A `string` meanwhile, does not allow you to obtain a pointer to the >>> underlying storage and that's what makes it "immutable". And that does >>> indeed mean that if you pass a `string` value around, that can't lead to >>> data races, while passing a `[]byte` around *might*. >>> >>> But for this case, it doesn't really matter whether or not the field is >>> a `string` or a `[]byte` or an `int`: Because the "mutable" type is the >>> `*URL`. Which represents a reference to some underlying `URL` variable, >>> that you can then mutate. The race happens because you have a method on a >>> pointer that mutates a field - *regardless* of the type of that field. >>> >>> I don't know if that helps, it's a bit subtle. >>> >>> On Mon, Mar 25, 2024 at 1:35 PM 'Lirong Wang' via golang-nuts < >>> golan...@googlegroups.com> wrote: >>> >>>> Wow, i am from other language and i thought `string` is immutable or >>>> something like that, so thread-safe for this operation. learned >>>> something new!!! Thanks >>>> On Thursday, March 21, 2024 at 11:42:24 PM UTC+8 Ethan Reesor wrote: >>>> >>>>> I hadn't used the race detector before. I do see a race warning for >>>>> (*URL).String() among an embarrassing number of other results. I'm going >>>>> to >>>>> update (*URL).String() to use atomic.Pointer to remove the race. >>>>> >>>>> Thanks, >>>>> Ethan >>>>> >>>>> On Thu, Mar 21, 2024 at 8:59 AM 'Axel Wagner' via golang-nuts < >>>>> golan...@googlegroups.com> wrote: >>>>> >>>>>> On Thu, Mar 21, 2024 at 2:48 PM 王李荣 <wanglir...@gmail.com> wrote: >>>>>> >>>>>>> hi Axel, >>>>>>> >>>>>>> is not modifying `u.memoize.str` thread-safe? the len and the data >>>>>>> point should become visible at same time? >>>>>>> >>>>>> >>>>>> What makes you think that? To be clear, there are no benign data >>>>>> races. Even a data-race on a variable smaller than a word is still a >>>>>> data-race, unless you do it holding a lock or using atomic instructions. >>>>>> But strings are *larger* than single words. >>>>>> >>>>>> To demonstrate that the effect I am talking about is real, look at >>>>>> this code: https://go.dev/play/p/LzRq9-OH-Xb >>>>>> >>>>>> >>>>>>> >>>>>>> 在2024年3月16日星期六 UTC+8 06:29:06<Axel Wagner> 写道： >>>>>>> >>>>>>>> Have you tried running the code with the race detector enabled? I >>>>>>>> suspect that you are concurrently modifying `u.memoize.str` by calling >>>>>>>> `u.String()` from multiple goroutines. And the non-zero length of the >>>>>>>> string header written by one goroutine becomes visible to the other >>>>>>>> one, >>>>>>>> before the modification to the data pointer. >>>>>>>> >>>>>>>> On Fri, Mar 15, 2024 at 11:15 PM Ethan Reesor <ethan....@gmail.com> >>>>>>>> wrote: >>>>>>>> >>>>>>>>> From this CI job >>>>>>>>> <https://gitlab.com/accumulatenetwork/accumulate/-/jobs/6398114923> >>>>>>>>> : >>>>>>>>> >>>>>>>>> panic: runtime error: invalid memory address or nil pointer >>>>>>>>> dereference >>>>>>>>> [signal SIGSEGV: segmentation violation code=0x1 addr=0x0 >>>>>>>>> pc=0x51d8b7] >>>>>>>>> goroutine 1589381 [running]: >>>>>>>>> strings.EqualFold({0xc000beec20?, 0x0?}, {0x0?, 0xacace7?}) >>>>>>>>> /usr/local/go/src/strings/strings.go:1111 +0x37 >>>>>>>>> >>>>>>>>> gitlab.com/accumulatenetwork/accumulate/pkg/url.(*URL).Equal(0xc000a74e40?, >>>>>>>>> 0xc00094c540) >>>>>>>>> /builds/accumulatenetwork/accumulate/pkg/url/url.go:472 +0x10c >>>>>>>>> >>>>>>>>> This is in a docker container based on the go:1.22 image, so the >>>>>>>>> panic appears to be happening here: >>>>>>>>> >>>>>>>>> func EqualFold(s, t string) bool { >>>>>>>>> // ASCII fast path >>>>>>>>> i := 0 >>>>>>>>> for ; i < len(s) && i < len(t); i++ { >>>>>>>>> sr := s[i] >>>>>>>>> tr := t[i] // <-- line 1111 >>>>>>>>> >>>>>>>>> (*URL).Equal >>>>>>>>> <https://gitlab.com/accumulatenetwork/accumulate/-/blob/5b1cb612d76d4163a101303e51a6fd352224cdab/pkg/url/url.go#L465> >>>>>>>>> : >>>>>>>>> >>>>>>>>> func (u *URL) Equal(v *URL) bool { >>>>>>>>> if u == v { >>>>>>>>> return true >>>>>>>>> } >>>>>>>>> if u == nil || v == nil { >>>>>>>>> return false >>>>>>>>> } >>>>>>>>> return strings.EqualFold(u.String(), v.String()) >>>>>>>>> } >>>>>>>>> >>>>>>>>> (*URL).String >>>>>>>>> <https://gitlab.com/accumulatenetwork/accumulate/-/blob/5b1cb612d76d4163a101303e51a6fd352224cdab/pkg/url/url.go#L240> >>>>>>>>> : >>>>>>>>> >>>>>>>>> func (u *URL) String() string { >>>>>>>>> if u.memoize.str != "" { >>>>>>>>> return u.memoize.str >>>>>>>>> } >>>>>>>>> >>>>>>>>> u.memoize.str = u.format(nil, true) >>>>>>>>> return u.memoize.str >>>>>>>>> } >>>>>>>>> >>>>>>>>> (*URL).format >>>>>>>>> <https://gitlab.com/accumulatenetwork/accumulate/-/blob/5b1cb612d76d4163a101303e51a6fd352224cdab/pkg/url/url.go#L189> >>>>>>>>> : >>>>>>>>> >>>>>>>>> func (u *URL) format(txid []byte, encode bool) string { >>>>>>>>> var buf strings.Builder >>>>>>>>> // ... write to the builder >>>>>>>>> return buf.String() >>>>>>>>> } >>>>>>>>> >>>>>>>>> How is this possible? Based on `addr=0x0` in the panic I think >>>>>>>>> this is a nil pointer panic, as opposed to some other kind of >>>>>>>>> segfault. The >>>>>>>>> only way I can reproduce panic-on-string-index is with >>>>>>>>> `(*reflect.StringHeader)(unsafe.Pointer(&s)).Data >>>>>>>>> = 0`, but I don't see how that can be happening here. I'm saving the >>>>>>>>> string >>>>>>>>> but I'm not doing anything weird with it. And the string header is a >>>>>>>>> value >>>>>>>>> type so code that manipulates the returned string shouldn't modify the >>>>>>>>> original. And I'm definitely not doing any kind of unsafe string >>>>>>>>> manipulation like that in my code, anywhere. The only reference to >>>>>>>>> unsafe >>>>>>>>> anywhere in my code is for parameters for calling GetDiskFreeSpaceExW >>>>>>>>> (Windows kernel32.dll call). >>>>>>>>> >>>>>>>>> -- >>>>>>>>> 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. >>>>>>>>> To view this discussion on the web visit >>>>>>>>> https://groups.google.com/d/msgid/golang-nuts/d6f6bb75-45e9-4a38-9bbd-d332e7f3e57cn%40googlegroups.com >>>>>>>>> <https://groups.google.com/d/msgid/golang-nuts/d6f6bb75-45e9-4a38-9bbd-d332e7f3e57cn%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 golang-nuts...@googlegroups.com. >>>>>>> To view this discussion on the web visit >>>>>>> https://groups.google.com/d/msgid/golang-nuts/31f77ff2-cf11-4b3e-9b14-874b6cc41da3n%40googlegroups.com >>>>>>> <https://groups.google.com/d/msgid/golang-nuts/31f77ff2-cf11-4b3e-9b14-874b6cc41da3n%40googlegroups.com?utm_medium=email&utm_source=footer> >>>>>>> . >>>>>>> >>>>>> -- >>>>>> >>>>> You received this message because you are subscribed to a topic in the >>>>>> Google Groups "golang-nuts" group. >>>>>> To unsubscribe from this topic, visit >>>>>> https://groups.google.com/d/topic/golang-nuts/Dgy0fyb4Shw/unsubscribe >>>>>> . >>>>>> To unsubscribe from this group and all its topics, send an email to >>>>>> golang-nuts...@googlegroups.com. >>>>>> To view this discussion on the web visit >>>>>> https://groups.google.com/d/msgid/golang-nuts/CAEkBMfG8v0qO_NP4PipEBL%3Dd_Ase9ntWi4EL1dQE_6ubeZQnww%40mail.gmail.com >>>>>> <https://groups.google.com/d/msgid/golang-nuts/CAEkBMfG8v0qO_NP4PipEBL%3Dd_Ase9ntWi4EL1dQE_6ubeZQnww%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 golang-nuts...@googlegroups.com. >>>> >>> To view this discussion on the web visit >>>> https://groups.google.com/d/msgid/golang-nuts/65400cce-b60c-4bb0-97a7-a963c6621098n%40googlegroups.com >>>> <https://groups.google.com/d/msgid/golang-nuts/65400cce-b60c-4bb0-97a7-a963c6621098n%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 golang-nuts+unsubscr...@googlegroups.com. >> To view this discussion on the web visit >> https://groups.google.com/d/msgid/golang-nuts/a0294b1b-ce7a-4b68-b462-d63db8d58ce6n%40googlegroups.com >> <https://groups.google.com/d/msgid/golang-nuts/a0294b1b-ce7a-4b68-b462-d63db8d58ce6n%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 golang-nuts+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/CAEkBMfE-ZqX4RqcbfPDp7v4xL48B-pk%3Dc1UVn4eH7sA4jeM1Ng%40mail.gmail.com.
