Golang, goroutines : panic: runtime error: invalid memory address

Question

I'm rather new in golang, and trying to understand main principles and write gouroutines based code, using chanels.

In others langs that i was using there were no such instruments, and i wonder getting such an errors like panic...

my code:

package main

import "fmt"
import (
    "time"
)
type Work struct {
    x,y,z int
}

func worker(in <-chan *Work, out chan<- *Work){
    for w := range in {
        w.z = w.x + w.y
        time.Sleep(time.Duration(w.z))
        out <-w
    }
}

func sendWork(in chan <- *Work){
    var wo *Work
    wo.x, wo.y, wo.z = 1,2,3
    in <- wo
    in <- wo
    in <- wo
    in <- wo
    in <- wo
}

func receiveWork(out <-chan *Work ) []*Work{
    var  slice []*Work
    for el := range out {
        slice = append(slice, el)
    }
    return slice
}

func main() {
    in, out := make(chan *Work), make(chan *Work)
    for i := 0; i<3; i++{
        go worker(in, out)
    }

    go sendWork(in)

    data := receiveWork(out)

    fmt.Printf("%v", data)
}

But at the terminal i got this :

panic: runtime error: invalid memory address or nil pointer dereference
[signal 0xc0000005 code=0x1 addr=0x0 pc=0x401130]

goroutine 8 [running]:
main.sendWork(0xc0820101e0)
        C:/temp/gocode/src/helloA/helloA.go:21 +0x20
created by main.main
        C:/temp/gocode/src/helloA/helloA.go:43 +0xe4

goroutine 1 [chan receive]:
main.receiveWork(0xc082010240, 0x0, 0x0, 0x0)
        C:/temp/gocode/src/helloA/helloA.go:31 +0x80
main.main()
        C:/temp/gocode/src/helloA/helloA.go:45 +0xf2

goroutine 5 [chan receive]:
main.worker(0xc0820101e0, 0xc082010240)
        C:/temp/gocode/src/helloA/helloA.go:12 +0x55
created by main.main
        C:/temp/gocode/src/helloA/helloA.go:40 +0xaf

goroutine 6 [runnable]:
main.worker(0xc0820101e0, 0xc082010240)
        C:/temp/gocode/src/helloA/helloA.go:11
created by main.main
        C:/temp/gocode/src/helloA/helloA.go:40 +0xaf

goroutine 7 [runnable]:
main.worker(0xc0820101e0, 0xc082010240)
        C:/temp/gocode/src/helloA/helloA.go:11
created by main.main
        C:/temp/gocode/src/helloA/helloA.go:40 +0xaf

How can i determine where is the problem, and how can i close the gouroutines nicely, not to left them as processes...

p.s. Forgive me my noob questions. please

Answer 1

The nil dereference:
You are trying to access a struct referenced by a pointer, but that pointer has not been set to an instance of that struct. You have to declare a struct that you can point the pointer to.

The error first appears here:

wo.x, wo.y, wo.z = 1,2,3

where you try to write to the object pointed to by wo. But the pointer is nil here; it's not actually pointing to an instance of Work. We have to create that instance, so we can point to it.

The nil value for a pointer to a struct is nil. If you don't declare a struct instance for it to point to, it points to nil.

var wo *Work

declares wo as a pointer of type Work to nil.

 var wo = &Work{}

declares wo as a pointer of type Work to a new instance of Work.

Or you can use the shorter syntax:

wo := &Work{}

As for the deadlock:

When we close a channel, range loops over that channel will exit. In func worker we range over a channel. When this channel is closed, the worker(s) will exit.

In order to wait for all the workers to finish processing, we use a sync.WaitGroup. This is an easy way to wait for a group of goroutines to finish running before continuing.

First you tell the waitgroup how many goroutines it should wait for.

wg.Add(3)

Then you wait:

wg.Wait()

until all of the goroutines have called

wg.Done()

which they do when they are done executing.

In this case, we need to close the output channel when all workers are done executing, so that the func receiveWork can exit its for range loop. We can do this by starting a new goroutine for this task:

go func() {
    wg.Wait()
    close(out)
}()

This is the whole file, after these edits:

package main

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

type Work struct {
    x, y, z int
}

func worker(in <-chan *Work, out chan<- *Work, wg *sync.WaitGroup) {
    for w := range in {
        w.z = w.x + w.y
        time.Sleep(time.Duration(w.z))
        out <- w
    }
    wg.Done() // this worker is now done; let the WaitGroup know.
}

func sendWork(in chan<- *Work) {
    wo := &Work{x: 1, y: 2, z: 3} // more compact way of initializing the struct
    in <- wo
    in <- wo
    in <- wo
    in <- wo
    in <- wo
    close(in) // we are done sending to this channel; close it
}

func receiveWork(out <-chan *Work) []*Work {
    var slice []*Work
    for el := range out {
        slice = append(slice, el)
    }
    return slice
}

func main() {
    var wg sync.WaitGroup
    in, out := make(chan *Work), make(chan *Work)
    wg.Add(3) // number of workers
    for i := 0; i < 3; i++ {
        go worker(in, out, &wg)
    }

    go sendWork(in)

    go func() {
        wg.Wait()
        close(out)
    }()

    data := receiveWork(out)

    fmt.Printf("%v", data)
}

Which outputs:

[0x104382f0 0x104382f0 0x104382f0 0x104382f0 0x104382f0]

which is probably not what you expected. It does however highlight one problem with this code. More on that later.

If you want to print the contents of the structs, you could either stop using pointers to Work, or loop over the elements of the slice and print them one-by-one, like this:

for _, w := range data {
    fmt.Printf("%v", w)
}

which outputs:

&{1 2 3}&{1 2 3}&{1 2 3}&{1 2 3}&{1 2 3}

Go doesn't follow pointers more than one step down when printing, to avoid infinite recursion, so you have to do this manually.

Race condition:

Since you are sending pointers to the same instance of *Work multiple times down the channel, that same instance is being accessed by multiple goroutines at the same time without synchronization. What you probably want is to stop using pointers, and use values. Work instead of *Work.

If you want to use pointers, maybe because Work is actually really big, you probably want to make multiple instances of *Work so you only ever send it to one goroutine.

Here's what the go race detector has to say about the code:

C:/Go\bin\go.exe run -race C:/gopath/src/github.com/drathier/scratchpad/go/main/main.go
[0xc0820403c0 0xc0820403c0 0xc0820403c0 0xc0820403c0 0xc0820403c0]==================
WARNING: DATA RACE
Write by goroutine 6:
  main.worker()
      C:/gopath/src/github.com/drathier/scratchpad/go/main/main.go:15 +0x8a

Previous write by goroutine 8:
  main.worker()
      C:/gopath/src/github.com/drathier/scratchpad/go/main/main.go:15 +0x8a

Goroutine 6 (running) created at:
  main.main()
      C:/gopath/src/github.com/drathier/scratchpad/go/main/main.go:45 +0x10c

Goroutine 8 (running) created at:
  main.main()
      C:/gopath/src/github.com/drathier/scratchpad/go/main/main.go:45 +0x10c
==================
Found 1 data race(s)
exit status 66

At this row:

w.z = w.x + w.y

all goroutines are modifying w.z concurrently, so if they try to write different values to w.z, there's no telling what value actually end up in there. Once again, this is easily fixed by creating multiple instances of *Work, or by using values instead of pointers: Work.