Yes, race conditions are definitely common. Thank you for pointing out the race condition in that example!

That code is intentionally simplified to make a point about the scheduling gotcha :-) The idea there was to show that a tight loop in one goroutine ( where the scheduler doesn't have an opportunity to execute) can prevent other goroutines from executing.

Why does this race?

After setting done = true, is there no guarantee that the goroutine will complete before the for loop ends?

there's no guarantee, they are running potentially concurrently. So the same variable is accessed from two goroutines with no mutex.

The assignment isn't guaranteed to be an atomic operation.

But there are no concurrent writers to the bool, the `true` setting should thus "eventually succeed" and reading the bool must resolve to either false or true at every point. Sure: maybe the first read happening concurrently with the write won't read true yet but surely the next one or the one after must?

Of course synchronization in-general is a must. But using a bool here with one writer and continuous-reading-until is probably the least-dangerous-of-possible-examples.. even a number or anything slicish incl strings combined with an operator other than equals would, in the same code-snippet length, make the dangers illustrable and live-observable when running even without `-race` =)

> Sure: maybe the first read happening concurrently with the write won't read true yet but surely the next one or the one after must?

You are expecting the read operation to be done either before or after the write, which would be the case if atomicity was guaranteed. There might be ugly intermediate states, or maybe there aren't, but don't know because there are no guarantees.

See also: http://preshing.com/20130618/atomic-vs-non-atomic-operations...

> There might be ugly intermediate states

In the general case: absolutely! In the above very specific setup as I outlined it (one bool, a single writer, a simple-waiting-for-that-single-write-loop) --- not. (Hence my point that another equally-tiny snippet might make the issue much more locally-reproducable-and-experienceable even without `-race`.)

What should the code be instead?

There are many possible fixes. The most obvious is to wrap it in a mutex (assuming 'done' is standing in for much more complex logic):

  func main() {  
      doneMutex := sync.Mutex{}
      done := false
  
      go func(){
          doneMutex.Lock()
          defer doneMutex.Unlock()
          done = true
      }()

      getDone := func() bool {
        doneMutex.Lock()
        defer doneMutex.Unlock()
        return done
      }
  
      for !getDone() {
          runtime.Gosched()
      }
      fmt.Println("done!")
  }

Another option would be to use a channel in place of done.

Effectively, the lesson here is that go doesn't have concurrently safe types (e.g. no concurrent map) nor generics to create them, so all concurrent code must be built around the builtin generic concurrent-safe type (channels) or must make careful use of mutexes / the atomic package / etc.

Idiomatic concurrent go either is built around channels or mutexes, and in either case the compiler won't tell you if you messed up, only the runtime race detector and testing will save you.

Version 1.9 introduced the sync.Map https://golang.org/pkg/sync/#Map

Would using a sync.WaitGroup and closure be wrong? Is mutex better for shared memory?

sync.Map is NOT a good solution for all concurrent map use cases. Please use your best judgement for the well documented trade-offs:

>The Map type is optimized for two common use cases: (1) when the entry for a given key is only ever written once but read many times, as in caches that only grow, or (2) when multiple goroutines read, write, and overwrite entries for disjoint sets of keys. In these two cases, use of a Map may significantly reduce lock contention compared to a Go map paired with a separate Mutex or RWMutex.

The fix that's probably closest to the author's intent is to use an atomic. There's no atomic boolean in the language, but if you store 0 or 1 in an int32 it works.