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/CAEkBMfGad5%2Brq1OHoehBXw%3D-xSaVHwxwqZ2_Vp%3D2XJEuv-0etg%40mail.gmail.com.
