Common Concurrent Programming Mistakes

No Synchronizations When Synchronizations Are Needed

• First, the read of b in the main goroutine and the write of b in the new goroutine might cause data races.
• Second, the condition b == true can't ensure that a != nil in the main goroutine. Compilers and CPUs may make optimizations by reordering instructions in the new goroutine, so the assignment of b may happen before the assignment of a at run time, which makes that slice a is still nil when the elements of a are modified in the main goroutine.

package main

import (
	"time"
	"runtime"
)

func main() {
	var a []int // nil
	var b bool  // false

	// a new goroutine
	go func () {
		a = make([]int, 3)
		b = true // write b
	}()

	for !b { // read b
		time.Sleep(time.Second)
		runtime.Gosched()
	}
	a[0], a[1], a[2] = 0, 1, 2 // might panic
}

package main

func main() {
	var a []int = nil
	c := make(chan struct{})

	go func () {
		a = make([]int, 3)
		c <- struct{}{}
	}()

	<-c
	// The next line will not panic for sure.
	a[0], a[1], a[2] = 0, 1, 2
}

Use time.Sleep Calls to Do Synchronizations

package main

import (
	"fmt"
	"time"
)

func main() {
	var x = 123

	go func() {
		x = 789 // write x
	}()

	time.Sleep(time.Second)
	fmt.Println(x) // read x
}

package main

import (
	"fmt"
	"time"
)

func main() {
	var n = 123

	c := make(chan int)

	go func() {
		c <- n + 0
	}()

	time.Sleep(time.Second)
	n = 789
	fmt.Println(<-c)
}

...
	tmp := n
	go func() {
		c <- tmp
	}()
...

Leave Goroutines Hanging

• a goroutine tries to receive a value from a channel which no more other goroutines will send values to.
• a goroutine tries to send a value to nil channel or to a channel which no more other goroutines will receive values from.
• a goroutine is dead locked by itself.
• a group of goroutines are dead locked by each other.
• a goroutine is blocked when executing a select code block without default branch, and all the channel operations following the case keywords in the select code block keep blocking for ever.

func request() int {
	c := make(chan int)
	for i := 0; i < 5; i++ {
		i := i
		go func() {
			c <- i // 4 goroutines will hang here.
		}()
	}
	return <-c
}

func request() int {
	c := make(chan int)
	for i := 0; i < 5; i++ {
		i := i
		go func() {
			select {
			case c <- i:
			default:
			}
		}()
	}
	return <-c // might hang here
}

Copy Values of the Types in the sync Standard Package

import "sync"

type Counter struct {
	sync.Mutex
	n int64
}

// This method is okay.
func (c *Counter) Increase(d int64) (r int64) {
	c.Lock()
	c.n += d
	r = c.n
	c.Unlock()
	return
}

// The method is bad. When it is called,
// the Counter receiver value will be copied.
func (c Counter) Value() (r int64) {
	c.Lock()
	r = c.n
	c.Unlock()
	return
}

Call the sync.WaitGroup.Add Method at Wrong Places

package main

import (
	"fmt"
	"sync"
	"sync/atomic"
)

func main() {
	var wg sync.WaitGroup
	var x int32 = 0
	for i := 0; i < 100; i++ {
		go func() {
			wg.Add(1)
			atomic.AddInt32(&x, 1)
			wg.Done()
		}()
	}

	fmt.Println("Wait ...")
	wg.Wait()
	fmt.Println(atomic.LoadInt32(&x))
}

...
	for i := 0; i < 100; i++ {
		wg.Add(1)
		go func() {
			atomic.AddInt32(&x, 1)
			wg.Done()
		}()
	}
...

Use Channels as Futures/Promises Improperly

doSomethingWithFutureArguments(<-fa(), <-fb())

ca, cb := fa(), fb()
doSomethingWithFutureArguments(<-ca, <-cb)

Close Channels Not From the Last Active Sender Goroutine

Do 64-bit Atomic Operations on Values Which Are Not Guaranteed to Be 8-byte Aligned

Not Pay Attention to Too Many Resources Are Consumed by Calls to the time.After Function

import (
	"fmt"
	"time"
)

// The function will return if a message
// arrival interval is larger than one minute.
func longRunning(messages <-chan string) {
	for {
		select {
		case <-time.After(time.Minute):
			return
		case msg := <-messages:
			fmt.Println(msg)
		}
	}
}

func longRunning(messages <-chan string) {
	timer := time.NewTimer(time.Minute)
	defer timer.Stop()

	for {
		select {
		case <-timer.C: // expires (timeout)
			return
		case msg := <-messages:
			fmt.Println(msg)

			// This "if" block is important.
			if !timer.Stop() {
				<-timer.C
			}
		}

		// Reset to reuse.
		timer.Reset(time.Minute)
	}
}

func longRunning(messages <-chan string) {
	timer := time.NewTimer(time.Minute)
	// defer timer.Stop() // become needless since Go 1.23

	for {
		select {
		case <-timer.C:
			return
		case msg := <-messages:
			fmt.Println(msg)
		}

		timer.Reset(time.Minute)
	}
}

Use time.Timer Values Incorrectly

• the Stop method of a *Timer value returns false if the corresponding Timer value has already expired or been stopped. If we know the Timer value has not been stopped yet, and the Stop method returns false, then the Timer value must have already expired.
• before Go 1.23, after a Timer value is stopped, its C channel field can only contain most one timeout notification.
• before Go 1.23, we should take out the timeout notification, if it hasn't been taken out, from a timeout Timer value after the Timer value is stopped and before resetting and reusing the Timer value. This is the meaningfulness of the if code block in the example in the last section.

package main

import (
	"fmt"
	"time"
)

func main() {
	start := time.Now()
	timer := time.NewTimer(time.Second/2)
	select {
	case <-timer.C:
	default:
		// Most likely go here.
		time.Sleep(time.Second)
	}
	// Potential data race in the next line.
	timer.Reset(time.Second * 10)
	<-timer.C
	fmt.Println(time.Since(start)) // about 1s
}