Go improper struct initialization?

Question

While coding I encountered a problem. When I use method of inner struct in goroutine, I can't see inner state like in this code.

package main

import (
    "fmt"
    "time"
)

type Inner struct {
    Value int
}

func (c Inner) Run(value int) {
    c.Value = value
    for {
        fmt.Println(c.Value)
        time.Sleep(time.Second * 2)
    }

}

type Outer struct {
    In Inner
}

func (c Outer) Run()  {
    go c.In.Run(42)

    for {
        time.Sleep(time.Second)
        fmt.Println(c.In)
    }
}

func main()  {
    o := new(Outer)
    o.Run()
}

Program printing:

from inner:  {42}
from outer:  {0}
from outer:  {0}
from inner:  {42}
from outer:  {0}
from inner:  {42}
from outer:  {0}
from outer:  {0}

Maybe it's pointer problem, but I don't know how resolve it.

icza · Accepted Answer · 2016-05-05 08:35:59Z

The most obvious error in your code is that Inner.Run() has a value-receiver, which means it gets a copy of the Inner type. When you modify this, you modify the copy, and the caller won't see any change on the Inner value.

So first modify it to have a pointer-receiver:

func (c *Inner) Run(value int) {
    // ...
}

If a method has a pointer-receiver, the address (pointer) of the value the method is called on will be passed to the method. And inside the method you will modify the pointed value, not the pointer. The pointer points to the same value that is present at the caller, so the same value is modified (and not a copy).

This change alone may make your code work. However, the output of your program is non-deterministic because you modify a variable (field) from one goroutine, and you read this variable from another goroutine too, so you must synchronize access to this field in some way.

One way to synchronize access is using sync.RWMutex:

type Inner struct {
    m     *sync.RWMutex
    Value int
}

When you create your Outer value, initialize this mutex:

o := new(Outer)
o.In.m = &sync.RWMutex{}

Or in one line:

o := &Outer{In: Inner{m: &sync.RWMutex{}}}

And in Inner.Run() lock when you access the Inner.Value field:

func (c *Inner) Run(value int) {
    c.m.Lock()
    c.Value = value
    c.m.Unlock()

    for {
        c.m.RLock()
        fmt.Println(c.Value)
        c.m.RUnlock()
        time.Sleep(time.Second * 2)
    }
}

And you also have to use the lock when you access the field in Outer.Run():

func (c Outer) Run() {
    go c.In.Run(42)

    for {
        time.Sleep(time.Second)
        c.In.m.RLock()
        fmt.Println(c.In)
        c.In.m.RUnlock()
    }
}

Note:

Your example only changes Inner.Value once, in the beginning of Inner.Run. So the above code does a lot of unnecessary locks/unlocks which could be removed if the loop in Outer.Run() would wait until the value is set, and afterwards both goroutines could read the variable without locking. In general if the variable can be changed at later times too, the above presented locking/unlocking is required at each read/write.

Thanks for your resolve! Now I'm understand how it work. Now code work nice.

Endre Simo · Accepted Answer · 2016-05-05 06:53:34Z

The simplest way to resolve your issue is to use a pointer receiver in your Run function:

func (c *Inner) Run(value int) {
    out = make(chan int)
    c.Value = value
    for {
        fmt.Println(c.Value)
        time.Sleep(time.Second * 2)
    }
}

But another solution would be to use an out channel to which you can send the Inner struct value:

func (c Inner) Run(value int) {
    out = make(chan int)
    c.Value = value
    for {
        fmt.Println(c.Value)
        time.Sleep(time.Second * 2)
        out <- c.Value
    }
}

Then in a separate goroutine to receive back the sent value:

for{
    go func() {
        c.In.Run(42)
        <-out
        fmt.Println(out)
    }()
    time.Sleep(time.Second)       
}

Here is the full code:

package main

import (
    "fmt"
    "time"
)

type Inner struct {
    Value int
}

var out chan int

func (c Inner) Run(value int) {
    out = make(chan int)
    c.Value = value
    for {
        fmt.Println(c.Value)
        time.Sleep(time.Second * 2)
        out <- c.Value
    }
}

type Outer struct {
    In Inner
}

func (c Outer) Run()  {    

    for{
        go func() {
            c.In.Run(42)
            <-out
            fmt.Println(out)
        }()
        time.Sleep(time.Second)       
    }
}

func main()  {
    o := new(Outer)
    o.Run()
}

https://play.golang.org/p/Zt_NAsM98_

Collectives™ on Stack Overflow

Go improper struct initialization?

2 Answers 2

1 Comment

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

1 Comment

Comments

Your Answer

Sign up or log in

Post as a guest

Related