Verifying my mental model about goroutines and access to closure variables

[Manuel Kiessling]

Hello all,

I'm currently writing my first "serious" http server application with Go. I would like to make sure that I correctly grasp some of the conceptual implications related with it.

My server is, as far as I can see, basically your standard http.ListenAndServe thing, nothing special.

One of my handlers generates an id whenever there is a POST request to /sessions, like so:

var id int

func generateId() {

  id = id + 1

  return id

}

func CreateSession(res http.ResponseWriter, req *http.Request) {

  io.WriteString(res, strconv.Itoa(generateId()))

}

Now, first I made a mental note along the lines of "guess I need to manage the id generation through a channel in order to avoid a mess when concurrent http requests (== goroutines) access the id closure". But my tests showed that no such problem occured, the session id is incremented correctly no matter how many concurrent http requests I fire at POST /sessions (using ab).

Now my mental model is that id = id + 1 seems to be atomic - there is no copy of id's value "somewhere" at any time, it is incremented in place. To verify this, I rewrote the code to:

var id int

func generateId() {

  curr := id

  time.Sleep(1000 * time.Millisecond)

  id = curr + 1

  return id

}

which, as expected, led to problems, that is, concurrent http requests resulting in concurrent goroutines all accessing "id", incrementing it by 1 from an "old" value that is no longer true.

Is that more or less a correct mental model?

Regards,

Manuel

[Jim Whitehead II]

The memory model makes very minimal guarantees, and
http://golang.org/ref/mem will help you better understand those. The
moral of the story is you shouldn't rely on the assignment being
atomic.

A fairly standard way of dealing with this is to turn generateId into
a goroutine that sits in a simple loop that manages the counter:

http://play.golang.org/p/MAQnFBBgKA

Just one way you might think about it. The memory model won't
guarantee that an assignment is atomic in all cases, so its simpler to
ensure the correct synchronization (and leads to a nice design).

- Jim

[Jan Mercl]

On Thu, Aug 16, 2012 at 11:06 AM, Manuel Kiessling <man...@kiessling.net> wrote:

Is that more or less a correct mental model?

I think you've observed correctly what happens in the Go implementation you're using, where properly aligned, small enough variables (int qualifies AFAIK), may be and usually are updated atomically on any modern HW. However, the Go's memory model doesn't guarantee this to be reliably usable. The compiler is free to "hide" the updates (wrt other goroutines) to any variable as long as there's no synchronization performed, e.g. the updated value may actually exist in a register only, thus not visible after a context switch.

In short, such code will work, but doesn't have to work in general (think any other Go implementation). That's other way how to say that the code is invalid.

-j

[minux]

On Thu, Aug 16, 2012 at 5:06 PM, Manuel Kiessling <man...@kiessling.net> wrote:

var id int

func generateId() {

  id = id + 1

  return id

}

Now, first I made a mental note along the lines of "guess I need to manage the id generation through a channel in order to avoid a mess when concurrent http requests (== goroutines) access the id closure". But my tests showed that no such problem occured, the session id is incremented correctly no matter how many concurrent http requests I fire at POST /sessions (using ab).

Now my mental model is that id = id + 1 seems to be atomic - there is no copy of id's value "somewhere" at any time, it is incremented in place. To verify this, I rewrote the code to:

it's not atomic in the current gc amd64 implementation, and you can't assume that either

(please refer to Go Memory Model docs for details)

if you just want to maintain a counter, you can use sync/atomic.AddInt32 or a channel of ids (preferable).

[Manuel Kiessling]

Great, that was exactly the kind of implementation I was thinking of - until (wrongly) realizing that it doesn't seem to be necessary.

Thanks,

Manuel

[Peter]

Are you calling runtime.GOMAXPROCS(n) with n >1? Not doing this could result in the behaviour you witness, as your server would only allow 1 goroutine at a time to access id, making it look atomic.

[Manuel Kiessling]

Good point, I'll verify that.

Regards,

-- 

  Manuel

[⚛]

On Thursday, August 16, 2012 11:06:24 AM UTC+2, Manuel Kiessling wrote:

Now my mental model is that id = id + 1 seems to be atomic - there is no copy of id's value "somewhere" at any time, it is incremented in place.

The operation can be conceptually split into these steps:

1. Read id into a goroutine-private variable V

2. Increment V by 1

3. Write V to id

If goroutine1 is executing these steps, goroutine2 which is also executing these steps can interrupt goroutine1 between step 1 and 2, or between step 2 and 3.

Each goroutine executing these steps has its own V.

[Will Madison]

This is very very minor butl here's a small bug fix to +Jim Whitehead 's excellent example: http://play.golang.org/p/8bDLK-iWtp

(Simple oversight in that the inner goroutine never wrote to the "done" channel to inform the caller of its status)

- Will

[Shuai Lin]

My 2 cents:

Another key point is that the current scheduler is not preemptive.  With the current sheduler implementation, a goroutine would not be switched out by the scheduler until it encounters some blocking call, e.g, lock a mutex, or a blocking syscall.

If the scheduler is preemptive, there may still be race conditions with the op's code even with GOMAXPROC = 1.

Lin.

在 2012年8月16日星期四UTC+8下午11时54分32秒，Peter写道：