After spending many hours debugging Go code in production, I started noticing a pattern: the same bugs kept showing up in code reviews. They weren’t obvious syntax errors, but subtle details specific to Go. Unless you’re already very familiar with the language, these bugs can be difficult to spot at first glance. In this article, I’ll walk through the most common Go pitfalls I’ve seen repeatedly during code reviews. 1- Goroutine Leak A goroutine leak happens when a goroutine starts but never finishes, it stays alive for the lifetime of the process and keeps consuming resources. It can be caused by channel improper handling such as: Receiving from a channel that no one ever sends to, or that is never closed.Sending to a channel that has no receiver (unbuffered) or whose buffer is full, with no way for receivers to drain it Receiving from a channel that no one ever sends to, or that is never closed. Sending to a channel that has no receiver (unbuffered) or whose buffer is full, with no way for receivers to drain it This is a typical scenario describing goroutine leak with a channel. In the following example: worker runs an infinite loop.It blocks forever waiting on <-ch.main exits without closing the channel. worker runs an infinite loop. worker It blocks forever waiting on <-ch. <-ch main exits without closing the channel. main In a long-running service, this worker would never terminate, leading to goroutine leak. package main

import (
	"fmt"
	"time"
)

func worker(ch chan int) {
	for {
		v := <-ch
		fmt.Println("Received:", v)
	}
}

func main() {
	ch := make(chan int)

	go worker(ch)

	time.Sleep(2 * time.Second)
	fmt.Println("Main exiting")
} package main

import (
	"fmt"
	"time"
)

func worker(ch chan int) {
	for {
		v := <-ch
		fmt.Println("Received:", v)
	}
}

func main() {
	ch := make(chan int)

	go worker(ch)

	time.Sleep(2 * time.Second)
	fmt.Println("Main exiting")
} How to fix it: Close the channel in the main function once you’ve finished sending items. Then use range ch to read from the channel, the loop automatically exits when the channel is closed. range ch func worker(ch chan int) {
    for v := range ch { // exits automatically when channel is closed
        fmt.Println("Received:", v)
    }
    fmt.Println("Worker exiting")
}

func main() {
    ch := make(chan int)

    go worker(ch)

    ch <- 1
    ch <- 2

    close(ch) // signal completion

    time.Sleep(time.Second)
    fmt.Println("Main exiting")
} func worker(ch chan int) {
    for v := range ch { // exits automatically when channel is closed
        fmt.Println("Received:", v)
    }
    fmt.Println("Worker exiting")
}

func main() {
    ch := make(chan int)

    go worker(ch)

    ch <- 1
    ch <- 2

    close(ch) // signal completion

    time.Sleep(time.Second)
    fmt.Println("Main exiting")
} A goroutine must always have: Proper channel closing logic.Timeout or cancellation mechanism. (explained more below in the second example) Proper channel closing logic. Timeout or cancellation mechanism. (explained more below in the second example) 2- No Synchronization Between main and Goroutines When you start goroutines but don’t synchronize them with main, your program can exit before the goroutines finish executing. main In Go, the program terminates as soon as the main function returns — even if other goroutines are still running. main Interrupting goroutines can cause: Database writes may not finishHTTP requests may not completeLogs may not flushBackground jobs may be interrupted Database writes may not finish HTTP requests may not complete Logs may not flush Background jobs may be interrupted The main function has no visibility about the goroutines status. There is no sync between the main function and goroutines. main func processItems(items []string) {
    for _, item := range items {
        go func(i string) {  
            process(i)
        }(item)
    }
} func processItems(items []string) {
    for _, item := range items {
        go func(i string) {  
            process(i)
        }(item)
    }
} How to fix it: Add sync.WaitGroup to create synchronization between main function and goroutines. Add  context for proper life cycle management. Add sync.WaitGroup to create synchronization between main function and goroutines. sync.WaitGroup main Add  context for proper life cycle management. context context prevents leaks by giving goroutines a cancellation signal and a lifecycle boundary: context Cancellation propagation: when parent cancels,  all child goroutines stop.Timeouts / Deadlines: with context.WithTimeout, stuck I/O stops automatically. Cancellation propagation: when parent cancels,  all child goroutines stop. Timeouts / Deadlines: with context.WithTimeout, stuck I/O stops automatically. context.WithTimeout Now main waits until all goroutines finish before it exits. main func processItems(ctx context.Context, items []string) error {
    var wg sync.WaitGroup
    wg.Add(len(items))
    for _, item := range items {
        go func(i string) {
            defer wg.Done()
            select {
            case <-ctx.Done():
                return
            default:
                process(i)
            }
        }(item)
    }
    wg.Wait()
    return ctx.Err()
} func processItems(ctx context.Context, items []string) error {
    var wg sync.WaitGroup
    wg.Add(len(items))
    for _, item := range items {
        go func(i string) {
            defer wg.Done()
            select {
            case <-ctx.Done():
                return
            default:
                process(i)
            }
        }(item)
    }
    wg.Wait()
    return ctx.Err()
} 3- Connections Leak When you open many HTTP connections, closing the connection with defer is a good practice, except when you run it inside a loop like below, it won't get executed after each iteration as expected. It will only get executed when the main function returns, leading to connections leak, by keeping all the established connections open. for _, url := range urls {
    resp, _ := http.Get(url)
    defer resp.Body.Close() // BAD in loop
} for _, url := range urls {
    resp, _ := http.Get(url)
    defer resp.Body.Close() // BAD in loop
} How to fix it: always close HTTP connections with defer inside a closure function in a loop. That way closing a connection happens at the end of each iteration, guaranteeing all open connections are closed. defer for _, url := range urls {
    func() {
        resp, _ := http.Get(url)
        defer resp.Body.Close()
    }()
} for _, url := range urls {
    func() {
        resp, _ := http.Get(url)
        defer resp.Body.Close()
    }()
} 4- HTTP Requests Without Context By default, http.Get() has no timeout. The request can hang forever even if: http.Get() Client disconnects Parent request is canceled Shutdown signal received Client disconnects Parent request is canceled Shutdown signal received Your HTTP call keeps running anyway. This breaks graceful shutdown and request scoping, leading to: Exhausting goroutinesFilling connection pools Exhausting goroutines Filling connection pools func fetchData(url string) error {
    resp, err := http.Get(url)
    if err != nil {
        return err
    }
    defer resp.Body.Close()

    _, err = io.ReadAll(resp.Body)
    return err
} func fetchData(url string) error {
    resp, err := http.Get(url)
    if err != nil {
        return err
    }
    defer resp.Body.Close()

    _, err = io.ReadAll(resp.Body)
    return err
} How to fix it:  use http.NewRequestWithContext instead, it provides: http.NewRequestWithContext Timeout controlCancellation propagationAutomatic cleanup Timeout control Cancellation propagation Automatic cleanup func fetchData(ctx context.Context, url string) error {
   ctx, cancel := context.WithTimeout(ctx, 2*time.Second)

   defer cancel()

   req, err := http.NewRequestWithContext(ctx, "GET", url, nil)

   if err != nil {
     return err
   }

   resp, err := http.DefaultClient.Do(req)
   if err != nil {
        return err
   }
   defer resp.Body.Close()

   _, err = io.ReadAll(resp.Body)
    return err
} func fetchData(ctx context.Context, url string) error {
   ctx, cancel := context.WithTimeout(ctx, 2*time.Second)

   defer cancel()

   req, err := http.NewRequestWithContext(ctx, "GET", url, nil)

   if err != nil {
     return err
   }

   resp, err := http.DefaultClient.Do(req)
   if err != nil {
        return err
   }
   defer resp.Body.Close()

   _, err = io.ReadAll(resp.Body)
    return err
} 5- Copying Slices and Maps the Wrong Way Copying maps and slices is not as straightforward as copying other types in Go. Assignment does not create a copy; it creates a reference to the underlying map or slice a := []int{1, 2, 3}
b := a
b[0] = 99 a := []int{1, 2, 3}
b := a
b[0] = 99 fmt.Println(a)
[99 2 3] fmt.Println(a)
[99 2 3] m1 := map[string]int{"a": 1}
m2 := m1
m2["a"] = 42 m1 := map[string]int{"a": 1}
m2 := m1
m2["a"] = 42 fmt.Println(m1["a"])
42 fmt.Println(m1["a"])
42 This is how to properly copy a slice or a map: src := []int{1, 2, 3}
dst := make([]int, len(src))
copy(dst, src)

b := make(map[string]int, len(a))
for k, v := range a {
    b[k] = v
} src := []int{1, 2, 3}
dst := make([]int, len(src))
copy(dst, src)

b := make(map[string]int, len(a))
for k, v := range a {
    b[k] = v
} 6- Capturing Loop Variables by Reference By default, closures capture variables by reference from the surrounding scope. In the following example: for _, v:=range servers {
  go func(){
    fmt.Println(v)
  }
} for _, v:=range servers {
  go func(){
    fmt.Println(v)
  }
} The goroutines don’t capture the value of v at each iteration, they capture the same variable v, which is reused by the loop. value v v v Goroutines run concurrently, by the time a goroutine executes fmt.Println(v), the loop may have already advanced or even finished. fmt.Println(v) You may get: cache
cache
cache cache
cache
cache Instead of : web
db
cache web
db
cache How to fix it: freeze the value of the variable v, by passing it as parameter to the closure. Each goroutine now receives its own copy. v for -, v:=range servers {
  go func(s *Server){
    fmt.Println(s)
  }(v)
} for -, v:=range servers {
  go func(s *Server){
    fmt.Println(s)
  }(v)
} 7- Modifying a Slice of Structs by Copy When you have a slice of structs and you want to iterate through that slice to modify the structs. the variable s  in the for range loop is just a copy of each element in the slice:  s := servers[i], not a reference to the struct. Modifying the copy won't modify the struct itself. s for range s := servers[i] type Server struct{
  Name string
  Status bool
}
servers :=[]Server{{"web",false},{"db",false}}

for _,s := range servers {
  s.status = true
} type Server struct{
  Name string
  Status bool
}
servers :=[]Server{{"web",false},{"db",false}}

for _,s := range servers {
  s.status = true
} After the loop, servers is still the same: servers [{web false} {db false}] [{web false} {db false}] How to fix it: use the loop index instead to access the slice element. for i,_ := range servers {
	servers[i].status
} for i,_ := range servers {
	servers[i].status
} 8- Unmarshaling JSON into Maps When you unmarshal JSON into map[string]interface{}, the types are not reserved: map[string]interface{} All numbers become float64Strings become stringBooleans become boolNested objects become map[string]interface{}Arrays become []interface{} All numbers become float64 float64 Strings become string string Booleans become bool bool Nested objects become map[string]interface{} map[string]interface{} Arrays become []interface{} []interface{} The following example illustrates the bug: {
  "id": 123,
  "name": "web-server",
  "active": true
} {
  "id": 123,
  "name": "web-server",
  "active": true
} var data map[string]interface{}

jsonBytes := []byte(`{
  "id": 123,
  "name": "web-server",
  "active": true
}`)

err := json.Unmarshal(jsonBytes, &data)
if err != nil {
    panic(err)
}

id := data["id"].(int) // panic!
fmt.Println(id) var data map[string]interface{}

jsonBytes := []byte(`{
  "id": 123,
  "name": "web-server",
  "active": true
}`)

err := json.Unmarshal(jsonBytes, &data)
if err != nil {
    panic(err)
}

id := data["id"].(int) // panic!
fmt.Println(id) interface conversion: interface {} is float64, not int interface conversion: interface {} is float64, not int The best solution is to use struct instead: type Server struct {
    ID     int    `json:"id"`
    Name   string `json:"name"`
    Active bool   `json:"active"`
}
var s Server

err := json.Unmarshal(jsonBytes, &s)
if err != nil {
    panic(err)
}

fmt.Println(s.ID) // Safe, typed, clean type Server struct {
    ID     int    `json:"id"`
    Name   string `json:"name"`
    Active bool   `json:"active"`
}
var s Server

err := json.Unmarshal(jsonBytes, &s)
if err != nil {
    panic(err)
}

fmt.Println(s.ID) // Safe, typed, clean Try to use  map[string]interface{} only when the JSON structure is dynamic or when you don't know the schema. Otherwise, stick to structs. map[string]interface{} 9- Concurrent Access to Shared Variables Without Locks When multiple goroutines access the same variable without synchronization, you create a data race. This can cause data inconsistency. With maps specifically, Go detects unsafe concurrent map writes and panics:  fatal error: concurrent map writes fatal error: concurrent map writes count := 0
for i:=0; i<10; i++{
  go func(){
    count ++
  }()
}
time.sleep(time.second)
fmt.Println(count) count := 0
for i:=0; i<10; i++{
  go func(){
    count ++
  }()
}
time.sleep(time.second)
fmt.Println(count) How to fix it:  add mutual-exclusion lock sync.Mutex, this will protect the shared variable from race condition . sync.Mutex var mu sync.Mutex
count :=0

for i:=0; i<1000; i++{
  go func(){
    mu.lock()
    count ++
    mu.unlock()
  }
} var mu sync.Mutex
count :=0

for i:=0; i<1000; i++{
  go func(){
    mu.lock()
    count ++
    mu.unlock()
  }
} You can detect it with: go run -race main.go go run -race main.go Go will show warnings like: WARNING: DATA RACE WARNING: DATA RACE 10- Dereferencing a Nil pointer Dereferencing a nil pointer can cause a runtime panic: var p *int 
fmt.Println(*p) var p *int 
fmt.Println(*p) panic: runtime error: invalid memory address or nil pointer dereference panic: runtime error: invalid memory address or nil pointer dereference Simple fix is to assign any variable address to the pointer: x := 10
p := &x x := 10
p := &x Conclusion: None of these bugs are considered as "blockers". They compile and most of the time they also work. The real issue arises when they lead to performance bottlenecks and security vulnerabilities.

The code in this story is for educational purposes. The readers are solely responsible for whatever they build with it.

Top 10 Golang Bugs to Spot Fast in Code Reviews

About Author

Comments

TOPICS

THIS ARTICLE WAS FEATURED IN

Related Stories

Hidden Kubernetes Bad Practices Learned the Hard Way During Incidents

50 Shades of Lead Time: Measuring Each Part of the Development Process

A Code Review Checklist to Focus on the Important Parts

Best Practices, Guidelines and Tools for Code Reviews

Code Reviews: A Practice That Depends on Culture

Code Reviews: A Multi-Tool You Paid Too Much For and Now Hesitate to Throw Away

Hidden Kubernetes Bad Practices Learned the Hard Way During Incidents

50 Shades of Lead Time: Measuring Each Part of the Development Process

A Code Review Checklist to Focus on the Important Parts

Best Practices, Guidelines and Tools for Code Reviews

Code Reviews: A Practice That Depends on Culture

Code Reviews: A Multi-Tool You Paid Too Much For and Now Hesitate to Throw Away

Light-Mode

Classic

Newspaper

Minty

Dark-Mode

Neon Noir

Minty

HN StartUps