Go: doc: document that unsynchronized (channel send + channel close) is a data race

[deckarep]

Now that I'm aware of this behavior i'm only proposing that this be documented in Go's documentation. My rationale is that since channels are built to be threadsafe, I would have assumed all operations against them are threadsafe. ie, sending, receiving, closing.

What version of Go are you using (go version)? go version go1.11 darwin/amd64

Does this issue reproduce with the latest release? Yes

What operating system and processor architecture are you using (go env)?

GOHOSTARCH="amd64"
GOHOSTOS="darwin"

What did you do?

First off, my code was flawed but it exposed a data-race that I thought would not be possible. My assumption was that all operations on channels are thread-safe by design..at the worst I would have expected the code to panic.

• Sending on a channel (threadsafe) Well-understood
• Receiving on a channel (threadsafe) Well-understood
• Sending on a channel already closed (panics) Well-understood
• Closing a channel (not threadsafe) Huh?

https://stackoverflow.com/questions/47714470/go-race-condition-when-closing-a-go-channel

What did you expect to see?

I expected a panic to occur but instead this data-race shows up.

What did you see instead?

WARNING: DATA RACE
Read at 0x00c0000b4000 by goroutine 10:
  runtime.chansend()
      /usr/local/Cellar/go/1.11/libexec/src/runtime/chan.go:140 +0x0
  main.tx()
      /Users/deckarep/Desktop/channel_close_race/main.go:15 +0x66

Previous write at 0x00c0000b4000 by main goroutine:
  runtime.closechan()
      /usr/local/Cellar/go/1.11/libexec/src/runtime/chan.go:327 +0x0
  main.main()
      /Users/deckarep/Desktop/channel_close_race/main.go:38 +0x11f

[randall77]

It's reported as a data race, but in reality it's a "your send and close are unordered" error. What it means is that although in this execution the send came before the close, there's no observable ordering enforcing that - the next time you run, it only takes different choices by the scheduler to make the send come after the close and cause a panic.

[deckarep]

@randall77 - thanks for the explanation. I think that is an understandable reasoning on why the race detector trips. I'm just questioning if this behavior should be documented...if so where would such docs belong?

[randall77]

@deckarep: I don't really know where such documentation should live. We have docs about how to use the race detector, but I don't see any place to describe the kind of races it finds.

[agnivade]

@dvyukov

[dvyukov]

What @randall77 said.
It's not a /data race/ per se, but it's a bug worth detecting. It's both hard to discriminate this in race detector, and more of a one-off corner case, so race detector just reports this as it reports all other data races.

Re documenting, I don't think there is a single doc that all users of race detector has read before using it. Thus I don't think this can be solved with documentation. But we could mention this in https://golang.org/doc/articles/race_detector.html#Typical_Data_Races

[deckarep]

I’m happy to take a crack and write something up to start off the docs. Then I can consult the Go team to help with wording and accuracy.

[Loveuse]

I think https://golang.org/ref/mem under the Channel communication section might also be a good place to mention it since the send on the channel may happen before its closure.

[as]

It would be nice if the documentation also mentioned that this race is a symptom of ineffective producer/consumer designation, not just ordering quirks in the program.

[andybons]

@deckarep are you still interested in doing this?

[tpaschalis]

Hope it's okay to bring this up nearly a year after.

I'm looking to contribute for the first time, and documentation seems a safe and easy place to start. The following code still produces a data race.

func TestRace(t *testing.T) {
    var c = make(chan byte, 20)
    go func() {
        defer func() {
            if r := recover(); r == nil {
                t.Error("expected panic error")
            }
        }()
        for i := 0; i < 25; i++ {
            c <- byte(0)
        }
        t.Error("expected a panic")
    }()
    close(c)
}

I could draft a paragraph to document this on https://golang.org/doc/articles/race_detector.html#Typical_Data_Races and get some feedback. Does it sound okay to you?

[gopherbot]

Change https://golang.org/cl/196681 mentions this issue: doc : document a race condition caused by unordered send and close

[gopherbot]

Change https://golang.org/cl/219637 mentions this issue: doc: race condition in unsynchronized send/close