0

I've a strange race condition. The problem is that it occurs inside an object which is not existing yet.

Here is a demo code:

package main

import (
    //"fmt"
    "time"
)

type Object1 struct {
    A int
    B string
    C []int
    D *Object2
}

type Object2 struct {
    A int
}

func NewObject1() *Object1 {
    return &Object1{
        A: 1,
        B: "abc",
        C: []int{0, 1},
        D: &Object2{},
    }
}

func main() {
    list := []*Object1{}

    tempA := 0
    tempB := ""
    tempC := []int{}
    tempD := &Object2{}

    go func() {
        for {
            for _, object := range list {
                tempA = object.A
                tempB = object.B
                tempC = object.C
                tempD = object.D
            }
        }
    }()

    for {
        list = append(list, NewObject1())

        //fmt.Println("list", list)
        time.Sleep(1 * time.Second)
    }
}

If I run it with the -race flag - I get the warnings:

WARNING: DATA RACE
Read at 0x00c000094040 by goroutine 5:
  main.main.func1()
      /tmp/race.go:39 +0x84

Previous write at 0x00c000094040 by main goroutine:
  main.main()
      /tmp/race.go:21 +0x2a9

Goroutine 5 (running) created at:
  main.main()
      /tmp/race.go:36 +0x276
==================
==================
WARNING: DATA RACE
Read at 0x00c000094048 by goroutine 5:
  main.main.func1()
      /tmp/race.go:40 +0xbe

Previous write at 0x00c000094048 by main goroutine:
  main.main()
      /tmp/race.go:22 +0x2ca

Goroutine 5 (running) created at:
  main.main()
      /tmp/race.go:36 +0x276
==================
==================
WARNING: DATA RACE
Read at 0x00c000094058 by goroutine 5:
  main.main.func1()
      /tmp/race.go:41 +0x118

Previous write at 0x00c000094058 by main goroutine:
  main.main()
      /tmp/race.go:23 +0x341

Goroutine 5 (running) created at:
  main.main()
      /tmp/race.go:36 +0x276
==================
==================
WARNING: DATA RACE
Read at 0x00c000094070 by goroutine 5:
  main.main.func1()
      /tmp/race.go:42 +0x180

Previous write at 0x00c000094070 by main goroutine:
  main.main()
      /tmp/race.go:24 +0x3b8

Goroutine 5 (running) created at:
  main.main()
      /tmp/race.go:36 +0x276
==================

But how is that possible? Reading happens inside a goroutine and writing inside a NewObject1(). 4 errors for every Object1 field. NewObject1() has not created an object yet to append it to the list slice. So list during the reading should be empty or filled with a normal completed objects.

Step by step workflow in my mind:

  1. list is empty;
  2. you start to create new object1;
  3. list is still empty;
  4. you have created a new object and only then add it to the list;
  5. only now list has 1 element;
  6. reading happens.

I don't see a race condition here. If you think differently - please show your own workflow of how things happens.

Alexey
  • 2,582
  • 3
  • 13
  • 31
  • "has not created an object yet" --- you don't have synchronisation in your code, so there is no "happens-after" relationship established between different parts of your code, hence you cannot reason about temporal aspects of events in runtime. – zerkms Apr 02 '19 at 07:34
  • @Flimzy, because `NewObject1()` is creating/returning a new fresh object and it can't be used somewhere else before is created. `NewObject1()` is only creating it at that moment, isn't it? – Alexey Apr 02 '19 at 07:55
  • If you think differently - please explain more technically detailed – Alexey Apr 02 '19 at 07:56
  • @Flimzy, yes, but goroutine shouldn't be able to read an object which is not created yet?! Question is about race condition. Step by step workflow in my mind - 1) list is empty. 2) you start to create new object1. 3) list is still empty. 4) you have created a new object and only then add it to the list. 5) only now list has 1 element. 6) reading happens. Where you see a race condition? – Alexey Apr 02 '19 at 08:01
  • 1
    https://stackoverflow.com/questions/44152988/append-not-thread-safe – mkopriva Apr 02 '19 at 08:04
  • @Flimzy, write your workflow which shows a race. Step by step. `yet` is BEFORE you add an element to the list – Alexey Apr 02 '19 at 08:07
  • @Alexey it does not happen BEFORE, but AFTER you added an element to the slice. – zerkms Apr 02 '19 at 08:08
  • @mkopriva even if it was thread-safe there still would be a data race. – zerkms Apr 02 '19 at 08:08
  • @zerkms, you mean empty struct is created first, added to the list, and only after that it's fields are being filled/written?? – Alexey Apr 02 '19 at 08:11
  • 2
    @Alexey you don't synchronise access to memory, there are no any guarantees. It makes no sense to discuss a data race: the behaviour is undefined and cannot be reasoned about. – zerkms Apr 02 '19 at 08:12
  • I just wish to understand. It seems that fields writing happens AFTER the struct is added to the list. Is that possible? Because I didn't want that in my code – Alexey Apr 02 '19 at 08:16
  • @Alexey you cannot tell when write **actually happened** – zerkms Apr 02 '19 at 08:16
  • @zerkms, by 'it seems' I mean what race detector shows. This is a little confusing – Alexey Apr 02 '19 at 08:18
  • @Alexey race detector only shows you have non-synchronised read and write operations. It tells nothing about whether any data was written there by that moment or not. It only checks memory addresses, it does not care about the actual operations progress. – zerkms Apr 02 '19 at 08:19
  • @Alexey: just as zerkms says, data race situation is undefined behaviour. Anything can happen. From corrupted data to [nuclear missile launch](https://software.intel.com/en-us/blogs/2013/01/06/benign-data-races-what-could-possibly-go-wrong). Synchronize your reads and writes. – Sergio Tulentsev Apr 02 '19 at 08:21
  • 3
    @zerkms I was only trying to point out that rather than focusing on `NewObject1` and the `temp` variable assignments, the issue can be fixed by synchronizing the read/write of the slice. E.g. this https://play.golang.org/p/60bevT89WFf doesn't trigger the race messages. – mkopriva Apr 02 '19 at 08:23
  • 1
    Your proper work flow is: 1. list is empty. 2. read list anywhere from 0 to infinite times. 3. Simultaneously write to the list anywhere from 0 to infinite times. <-- data race 4. jump to 2 – Jonathan Hall Apr 02 '19 at 08:24

1 Answers1

5

Race detector detects that you concurrently read and write the same address in memory.

It is by definition a data race.

It does not matter when data was actually put to that address (and whether it was put there at all). What only matters is that you access the same memory in different goroutines without synchronisation, and one of those operations is a "write".

Both are not about Go, but are extremely quality resources:

  1. https://preshing.com/20120710/memory-barriers-are-like-source-control-operations/ - this and basically every other article in that blog is a gem.
  2. http://deadlockempire.github.io/ - a puzzle-like game that reveals nuances of synchronisation and concurrency issues
zerkms
  • 249,484
  • 69
  • 436
  • 539
  • Even if 1 hour is between writing the variable and reading it in different goroutines - race will be detected? – Alexey Apr 02 '19 at 08:23
  • @Alexey race detector may have false negatives: it may not detect something. But yes, it still is a data race: if you don't synchronise the second goroutine is not guaranteed to ever see the changed values. The synchronisation happens by explicit CPU instructions (memory barriers) and has nothing to do with how long it passed between the operations. – zerkms Apr 02 '19 at 08:24
  • Didn't expect that. So with goroutines every single variable must be synchronized?! And I thought only pointers like slices/maps are dangerous – Alexey Apr 02 '19 at 08:28
  • 2
    @Alexey The thing is, if you start 2 goroutines, one creates a value (and stores it in a variable), the other sleeps 1 hour and attempts to read the value from variable, without synchronization, there is no guarantee the creation will happen before the read. The goroutine scheduler will do its best, but only synchronization gives guarantee, "nothing" else. – icza Apr 02 '19 at 08:28
  • @icza, I see now. Just didn't expect. However, I'm sure Go will be able to do writing in 1 hour :) – Alexey Apr 02 '19 at 08:30
  • @Alexey also see the update with couple useful links – zerkms Apr 02 '19 at 08:31
  • 1
    @Alexey Yes, in practice the creation will not be delayed for an hour, but only synchronization gives guarantee. And race detector detects the lack of synchronization, the lack of guarantee. – icza Apr 02 '19 at 08:31
  • It's not Go though: a CPU may decide to not synchronise caches. Go should cooperate with CPU, but CPU alone is free to do a lot of weird things – zerkms Apr 02 '19 at 08:31
  • Probably last question, about the lack of guarantee to confirm - does this mean, that in my demo code I can get a situation, when the list already contains an `Object1`, but it's `object.A` is still `0`? – Alexey Apr 02 '19 at 08:37
  • @Alexey I'd say any value might be there: imagine a situation when memory was already allocated but was not initialised yet. – zerkms Apr 02 '19 at 08:40
  • You mean it could contain any bytes from previous object which was located at the same memory space? – Alexey Apr 02 '19 at 08:43
  • @Alexey yep, why not. Most likely operations would not be reordered SO dramatically, but theoretically - why not :shrug: – zerkms Apr 02 '19 at 08:43
  • My world was changed today, at least a little, thanks! – Alexey Apr 02 '19 at 08:45
  • @Alexey check the Jeff Preshing's blog (#1 in the answer). A lot of articles there cover low level CPU & Memory interactions, it's a very entertaining reading. – zerkms Apr 02 '19 at 08:46