golang / tour

[mirror] A Tour of Go
BSD 3-Clause "New" or "Revised" License
1.54k stars 521 forks source link

tour: section sync.Mutex is misguiding #655

Open BubbleBear opened 5 years ago

BubbleBear commented 5 years ago

Context: https://tour.golang.org/concurrency/9

with all due respect, the code example in sync.Mutex is so very wrong. i posted it here below. the section names sync.Mutex but the key that keeps it working is not sync.Mutex, but time.Sleep.

if you delete line:37 "time.Sleep(time.Second)", the output isn't 1000 anymore (when Value method is called, there are still Inc methods running or pending). while if you keep this line and delete all sync.Mutex related code, it works as well (perhaps it's because the virtual environment has only one CPU core).

though sync.Mutex doesn't make the Value method to block until all Inc methods done, the results above is way too misguiding.

package main

import (
    "fmt"
    "sync"
    "time"
)

// SafeCounter is safe to use concurrently.
type SafeCounter struct {
    v   map[string]int
    mux sync.Mutex
}

// Inc increments the counter for the given key.
func (c *SafeCounter) Inc(key string) {
    c.mux.Lock()
    // Lock so only one goroutine at a time can access the map c.v.
    c.v[key]++
    c.mux.Unlock()
}

// Value returns the current value of the counter for the given key.
func (c *SafeCounter) Value(key string) int {
    c.mux.Lock()
    // Lock so only one goroutine at a time can access the map c.v.
    defer c.mux.Unlock()
    return c.v[key]
}

func main() {
    c := SafeCounter{v: make(map[string]int)}
    for i := 0; i < 1000; i++ {
        go c.Inc("somekey")
    }

    time.Sleep(time.Second)
    fmt.Println(c.Value("somekey"))
}
BubbleBear commented 5 years ago
package main

import (
    "fmt"
    "sync"
)

func main() {
    c := counter{}
    wg := sync.WaitGroup{}

    for i := 0; i < 1000; i++ {
        wg.Add(1)

        go (func() {
            c.inc()
            defer wg.Done()
        })()
    }

    wg.Wait()

    fmt.Println(c.v)
}

type counter struct {
    v int
    mux sync.Mutex
}

func (c *counter) inc() {
    c.mux.Lock()
    c.v++
    c.mux.Unlock()
}

here is what i suppose to be right.

gauffer commented 4 years ago
package main

import (
  "fmt"
  "sync"
)

func main() {
  c := counter{}
  wg := sync.WaitGroup{}

  for i := 0; i < 1000; i++ {
      wg.Add(1)

      go (func() {
          c.inc()
          defer wg.Done()
      })()
  }

  wg.Wait()

  fmt.Println(c.v)
}

type counter struct {
  v int
  mux sync.Mutex
}

func (c *counter) inc() {
  c.mux.Lock()
  c.v++
  c.mux.Unlock()
}

here is what i suppose to be right.

Jesus f***** Christ! Man, thank you SO MUCH, I'v just saved a ton of time making sense of it.

also I love your code design. main above, then type, then foo