0

A seemingly clever trick to avoid locking in concurrent C code goes like this: I have a global variable ptr which points to a mystruct and I want to update that structure. So I'll allocate a new mystruct, fill the data in and only then I'll make the change visible to the world by pointing ptr to the new mystruct object.

This is incorrect as it depends on the ordering of writes and there's no guarantee that the write to ptr will become visible to other threads after all stores to the new mystruct have taken place. Therefore, the new mystruct object can be returned partially initialized.

My question is: can this happen in Go, too? I think it can, but I have to say I found The Go Memory Model a little incomprehensible.

I wrote a bit of Go code to test it out, but on my machine, the bad behavirour does not manifest itself:

package main

import (
    "fmt"
    "time"
)

type mystruct struct {
    a int
    b int
}

var (
    ptr *mystruct
    counter int
)

func writer() {
    for {
        counter += 1
        s := mystruct{a: counter, b: counter}
        ptr = &s
    }
}

func reader() {
    time.Sleep(time.Millisecond)
    for {
        if ptr.a != ptr.b {
            fmt.Println("Oh no, I'm so buggy!")
        }
    }
}

func main() {
    go writer()
    go reader()
    select {}
}

This of course proves nothing.

Can you please provide a very brief comparison of memory guarantees provided by Go's goroutines with (almost no guarantees) provided by a POSIX thread in C?

too honest for this site
  • 12,050
  • 4
  • 30
  • 52
David
  • 1,055
  • 8
  • 23
  • You might find this interesting / edifying: [Golang data races to break memory safety](http://blog.stalkr.net/2015/04/golang-data-races-to-break-memory-safety.html). Also: [Is it safe to read a function pointer concurrently without a lock?](https://stackoverflow.com/questions/41406501/is-it-safe-to-read-a-function-pointer-concurrently-without-a-lock/41407827#41407827) – icza Apr 05 '18 at 19:38
  • I can't answer the q. as posed--enumerate all the guarantees--without rewriting the mem-model doc and maybe getting something wrong. For what it's worth, the officially endorsed solution for publishing a struct like that is `atomic.Value`: https://golang.org/pkg/sync/atomic/#Value – twotwotwo Apr 05 '18 at 19:39
  • "A seemingly clever trick …" - that's not a trick, but common place. But it's not really that simple and can caouse a variety of missbehaviour. – too honest for this site Apr 05 '18 at 19:48
  • 1
    The code there is definitely racy, though. There's nothing there that could signal to the compiler it needs to order memory accesses a particular way, so it could keep s.a and s.b in registers after assigning to ptr if it wanted to. It happens to run those instructions in the right order, and your cpu arch made their effects visible in the right order, but since the code did not ask for any synchronization that's just luck. – twotwotwo Apr 05 '18 at 19:49
  • @Olaf Not sure what you're trying to say. Could you please elaborate a bit? If by "common place" you mean a common mistake, then I agree. By "seemingly clever trick" I meant that people actually think they're so great when they come up with something like that. (Happened to me, too.) Especially, what do you mean "It's not really that simple"? Your comment makes me suspect you've got some insight and it'd be great if you shared it! – David Apr 05 '18 at 19:56
  • It is common practice, but requires certain measures (mostly using atomics) to avoid e.g. UB. Oh, and please refrain from spamming tags. Just you mentioning C does not justify adding the tag. – too honest for this site Apr 05 '18 at 19:58
  • @twotwotwo "It is, of course, incorrect code in the Q, but helping people correct mistakes is part of what we do on SO." Yes, especially when the question is *not* about a particular piece of code, I *don't* ask for anybody to fix my code and I added it as a certificate of not being all that lazy and trying to prove something myself before bothering other people. :) – David Apr 05 '18 at 20:01
  • @Olaf Ad spamming, I assumed that the person answering my question would have deep insight in C to be able to compare it's memory model to Go's. Is it wrong to use that tag in that case? For the other part, I still don't quite get you (sorry). "It is common practice, but requires certain measures (mostly using atomics) to avoid e.g. UB." Doing that in C is obviously wrong and that's what I say in the question. The question was whether it's valid Go and, more broadly, I asked for major differences between Go and C as I come from the C world. – David Apr 05 '18 at 20:06
  • 1
    C has no specific memory model. So, no. The standard libaries implementation is not specified in the standard. Nor anything else. An implementation could easily use database keys as pointer values. And using two (or more) alternating objects between two consurrent tasks (in the wider sense) is not wrong _per se_, it just has to be implemented correctly. I have no idea how to explain "common practice/place" beyond what you find in a dictionary. If you try to compare certain features of two languages, it is most helpful to learn both. There are good textbooks about C. – too honest for this site Apr 05 '18 at 20:09
  • @Olaf Can we chat in the *C* room, please? – David Apr 05 '18 at 20:39

1 Answers1

2

The Go Memory Model

Version of May 31, 2014

Advice

If you must read the rest of this document to understand the behavior of your program, you are being too clever.

Don't be clever.


Introducing the Go Race Detector


I [David] wrote a bit of Go code to test it out.

Your Go program has data races. The results are undefined.

$ go run -race david.go
==================
WARNING: DATA RACE
Read at 0x000000596cc0 by goroutine 7:
  main.reader()
      /home/peter/gopath/src/david.go:29 +0x4b

Previous write at 0x000000596cc0 by goroutine 6:
  main.writer()
      /home/peter/gopath/src/david.go:22 +0xf8

Goroutine 7 (running) created at:
  main.main()
      /home/peter/gopath/src/david.go:37 +0x5a

Goroutine 6 (running) created at:
  main.main()
      /home/peter/gopath/src/david.go:36 +0x42
==================
==================
WARNING: DATA RACE
Read at 0x00c0000cc270 by goroutine 7:
  main.reader()
      /home/peter/gopath/src/david.go:29 +0x5b

Previous write at 0x00c0000cc270 by goroutine 6:
  main.writer()
      /home/peter/gopath/src/david.go:21 +0xd2

Goroutine 7 (running) created at:
  main.main()
      /home/peter/gopath/src/david.go:37 +0x5a

Goroutine 6 (running) created at:
  main.main()
      /home/peter/gopath/src/david.go:36 +0x42
==================
==================
WARNING: DATA RACE
Read at 0x00c0000cda38 by goroutine 7:
  main.reader()
      /home/peter/gopath/src/david.go:29 +0x7f

Previous write at 0x00c0000cda38 by goroutine 6:
  main.writer()
      /home/peter/gopath/src/david.go:21 +0xd2

Goroutine 7 (running) created at:
  main.main()
      /home/peter/gopath/src/david.go:37 +0x5a

Goroutine 6 (running) created at:
  main.main()
      /home/peter/gopath/src/david.go:36 +0x42
==================
<<SNIP>>

Your Go program: david.go:

package main

import (
    "fmt"
    "time"
)

type mystruct struct {
    a int
    b int
}

var (
    ptr     *mystruct
    counter int
)

func writer() {
    for {
        counter += 1
        s := mystruct{a: counter, b: counter}
        ptr = &s
    }
}

func reader() {
    time.Sleep(time.Millisecond)
    for {
        if ptr.a != ptr.b {
            fmt.Println("Oh no, I'm so buggy!")
        }
    }
}

func main() {
    go writer()
    go reader()
    select {}
}

Playground: https://play.golang.org/p/XKywmzrRRRw

peterSO
  • 158,998
  • 31
  • 281
  • 276
  • Is it possible I didn't know about go run -race? Thanks a lot! – David Apr 05 '18 at 19:45
  • Consider `atomic.Value` if you want to "publish" structs like that for any other goroutine to read: https://golang.org/pkg/sync/atomic/#Value -- sadly the memmodel docs don't actually discuss it, but examples and how it's used in the stdlib indicate that that's accepted usage. – twotwotwo Apr 05 '18 at 19:55