DataRace with slice, should i fix it or ignore it?

[Pelen Li]

I want to set a value with the index of the slice. I don't really care if there are multiple goroutines cover the value with each other, because the value is same.

Can i just ignore this DataRace Warning? I don't know if this will cause panic.

Here's my example:

I defined a structure with slice, and a Add() function for it. sample like this:
```go

package test_slice

type SliceObj struct {
    set []uint
}

func New(length int64) *SliceObj {
    return &SliceObj{
        set: make([]uint, length),
    }
}

func (b *SliceObj) Add(i uint) {
    b.set[i] = i
}

```

And then i make a main file to test it, like this:

```go

package main

import (
    "time"

    "test_slice"
)

func main() {
    s := test_slice.New(1000000)
    go func() {
        s.Add(10)
    }()
    s.Add(10)

    time.Sleep(3 * time.Second)
}

```

And data race is detected:

(I know the reason of this warning, but I don't know if I can ignore it)

==================
WARNING: DATA RACE
Write at 0x00c000180050 by goroutine 18:
  test_slice.(*SliceObj).Add()
      test_slice.go:27 +0x68
  main.main.func1()
      test.go:25 +0x36

Previous write at 0x00c000180050 by main goroutine:
  test_slice.(*SliceObj).Add()
      test_slice.go:27 +0xfd
  main.main()
      test.go:27 +0xcb

Goroutine 18 (running) created at:
  main.main()
      test.go:24 +0xca
==================
Found 1 data race(s)
exit status 66

[Axel Wagner]

I would fix it.

One particular reason to do so is that, even if it works fine, it adds noise to the race detector. If someone uses your package and they test *their* code using the race detector, they will get spurious race detector messages. The same goes, of course, for your own tests as well. If race detector warnings are expected, you can't really use it to find *other* races.

To me, this alone is a good enough reason to fix the warning.

--
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/0b0b0178-987f-42bc-b8ef-47516764ffcen%40googlegroups.com.

[Brian Candler]

Agreed. Depending on your application, maybe using sync.Map would be a drop-in replacement, especially if this is a sparse set.

[peterGo]

Pelen Li,

Always fix your data races. You should consider the results of data races as undefined.

Dmitry Vyukov, who implemented the Go race detector, once wrote an interesting article with the title: "Benign data races: what could possibly go wrong?"

https://twitter.com/dvyukov/status/288858957827682304

The Go Blog: Introducing the Go Race Detector
Dmitry Vyukov and Andrew Gerrand
26 June 2013

https://go.dev/blog/race-detector

Hans-J. Boehm wrote: "there is no reason to believe that a currently working program with “benign races” will continue to work when it is recompiled."

How to miscompile programs with “benign” data races
Hans-J. Boehm
HP Laboratories

https://www.usenix.org/legacy/event/hotpar11/tech/final_files/Boehm.pdf

Nonetheless many programmers clearly believe, along with [15] that certain kinds of data races can be safely ignored in practice because they will produce expected results with all reasonable implementations. Here we show that all kinds of C or C++ source-level “benign” races discussed in the literature can in fact lead to incorrect execution as a result of perfectly reasonable compiler transformations, or when the program is moved to a different hardware platform. Thus there is no reason to believe that a currently working program with “benign races” will continue to work when it is recompiled. Perhaps most surprisingly, this includes even the case of potentially concurrent writes of the same value by different threads.

And so on.

Peter

[peterGo]

Pelen Li,

Boehm covers your specific case: "there is no reason to believe that a currently working program with “benign races” will continue to work when it is recompiled. Perhaps most surprisingly, this includes even the case of potentially concurrent writes of the same value by different threads."

Peter

--
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/ai0eQtnas7A/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/2594a018-872a-4d54-8ede-c769042e99b5n%40googlegroups.com.

[Robert Engels]

The problem with this reasoning is that Go still does not have an explicit memory model with respect to happens before relationships - falling back on “whatever the current implementation behavior is”. This makes it more likely that any change in the compiler will need to continue to support the benign data races. 

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/CAOBWp8do4njo%2B56hb7PJ-4RdXxV%2B0KGUVwv9zRC%3Dgd3bm%2BSM_g%40mail.gmail.com.

[Robert Engels]

See issue 5045

On Feb 9, 2022, at 8:42 AM, Robert Engels <ren...@ix.netcom.com> wrote:



[Daniel Fava]

Since we are on the topic, may be interesting to someone:

https://dfava.github.io/papers/fava2020finding.pdf

https://dfava.github.io/thesis/dfava_thesis.pdf

I’m not so sure I agree with you saying “ whatever the current implementation behavior is”

Daniel

On Feb 9, 2022, at 3:45 PM, Robert Engels <ren...@ix.netcom.com> wrote:



To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/78D7693F-198F-4CDE-BEBE-C296D77111F3%40ix.netcom.com.

[Axel Wagner]

While I do agree that the race should be fixed (as I mentioned), I would like to point out that with

https://github.com/golang/go/issues/50859

the program posted by OP would have a well-defined, correct outcome. Specifically, the intent of that proposal is:

Reads and writes of values larger than a single machine word behave as multiple
machine-word-sized operations in an unspecified order.

Note that this means that races on multiword data structures can lead to
inconsistent values not corresponding to a single write. When the values depend
on the consistency of internal (pointer, length) or (pointer, type) pairs, as
is the case for interface values, maps, slices, and strings in most Go
implementations, such races can in turn lead to arbitrary memory corruption.

Note that acausal and “out of thin air” writes are disallowed: each read must
observe a value written by a preceding or concurrent write.

Of course, the outcome of the writes is not observed in OPs program. And if it *was* observed, there would have to be some synchronization to order the reads after at least one of the writes.

But multiple concurrent writes of the same value - even a multi-word value - would be defined to produce a value that is at least partly the written one.

Again, I think the race detector complaining is in and off itself a good enough reason to fix the race. But Go at least intends to do something reasonable in this case.

To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/CF1981EE-3A3F-4CEF-AFA2-C5248F89B07F%40gmail.com.

[jake...@gmail.com]

On Wednesday, February 9, 2022 at 9:14:52 AM UTC-5 peterGo wrote:

Pelen Li,

Always fix your data races. You should consider the results of data races as undefined.

Dmitry Vyukov, who implemented the Go race detector, once wrote an interesting article with the title: "Benign data races: what could possibly go wrong?"

https://twitter.com/dvyukov/status/288858957827682304

This article by Dmitry Vyukov was an "oldie but goodie". I have used it as reference in these kinds of discussions before. Unfortunately, AFAICT, the article is no longer available on Intel's site. I have been unable to locate an alternate location. If anyone knows where I can find it, I would appreciate the info.

[Ian Lance Taylor]

[ + dvyukov ]

[Dmitry Vyukov]

I've recovered it and updated the go/benign-race link.

[Dan Kortschak]

It is on the way back machine
https://web.archive.org/web/20150604005924/http://software.intel.com/en-us/blogs/2013/01/06/benign-data-races-what-could-possibly-go-wrong

[fge...@gmail.com]

If the updated "go/benign-race link" is a canonical url, could you
please share the prefix as well?
( I tried http://go.dev/go/benign-race and some other variations, but
these don't work. Archive.org link is working. )

On 2/10/22, 'Dmitry Vyukov' via golang-nuts

> --
> 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/CACT4Y%2BbWWEHRrK3hvMd0ndSE%3DcdyfhN8gyCYRBfuc6-xJSsSfg%40mail.gmail.com.
>

[Dmitry Vyukov]

On Fri, 11 Feb 2022 at 09:54, <fge...@gmail.com> wrote:
>
> If the updated "go/benign-race link" is a canonical url, could you
> please share the prefix as well?
> ( I tried http://go.dev/go/benign-race and some other variations, but
> these don't work. Archive.org link is working. )

Oh, sorry didn't realize this is an external thread. Here is full link:
https://docs.google.com/document/d/1WAT22FtFdMt43i3O8YrnlQTNTzZ3IoJBtu3P2DNRytg/edit?usp=sharing