Как решить проблему с DATA RACE?

Question

[Павел]

Суть проблемы в том, что я имею массив каких-то объектов (структур). У этой структуры есть какие-то свойства и несколько методов. Обходя по таймеру этот цикл я обращаюсь к свойствам, чтобы определить, нужно ли вызывать определенный метод этой структуры в отдельном потоке.
Приведу пример кода:

import (
	"fmt"
	"sync"
	"time"
)
// наша структура
type Loc struct {
	mutex   sync.Mutex
	is_sync bool
	Value   int
}
// метод, который говорит, можно ли запускать метод Request
func (this Loc) IsSync() bool {
	return this.is_sync
}
// основной запрос, который может длиться несколько секунд
func (this *Loc) Request(val int) {
	this.is_sync = true

	defer func() {
		this.is_sync = false
		fmt.Println("End")
	}()

	fmt.Println("Request...")

	this.Value = val
	time.Sleep(time.Second)
}

var list []Loc

func init() {
	list = append(list, Loc{})
}

func main() {
	c := time.Tick(time.Second * 2)

	for range c {
		for i := 0; i < len(list); i++ {
			if list[i].IsSync() { // если ещё не пришло время, то пропускаем
				continue
			}

			go list[i].Request(i)
		}
	}
}

При запуске данного кода с параметром "race" (go run -race test.go) получаю предупреждение "WARNING: DATA RACE", а именно ругается на обращение к методу IsSync в самом цикле.

Метод Request может выполняться до полуминуты, а обход массива нужен примерно каждые 2 секунды. Сразу оговорюсь, что метод IsSync условен. В действительности там более сложные проверки, но всегда возвращает да или нет.

Суть проблемы понимаю. То есть предупреждение в данном примере очевидно - в одно прекрасное время может произойти такая ситуация, когда поток и цикл могут одновременно обратиться к одному и тому же свойству.
Но как это исправить? Если добавить блокировку в метод IsSync, это не решит проблему.

func (this Loc) IsSync() bool {
	this.mutex.Lock()
	s := this.is_sync
	this.mutex.Unlock()
	return s
}

Answer 1

[uvelichitel]

Здесь два момента

• go list[i].Request(i)гарантировано инициирует поток, но вовсе не обязательно запускает и this.is_sync = true не исполняется. То есть может быть инициировано сколь угодно много горутин привязанных к одной Loc struct. Приоритет исполнения рутин в Go не детерменирован.
  
• Параметры в функцию передаются копированием значения, то есть вызов func (this Loc) IsSync() boolпровоцирует полное копирование Loc, который может быть в работе. Что бы избежать копирования передавайте ссылкой.
  

В рамках вашей архитетуры(что бы не менять код сильно) так должно работать

package main

import (
	"fmt"
	"sync/atomic"
	"time"
)

// наша структура
type Loc struct {
	is_sync *int32
	Value   int
}

// метод, который говорит, можно ли запускать метод Request
func (this *Loc) IsSync() bool {    //Передавайте параметр ссылкой, что бы избежать копирования
	return (atomic.LoadInt32(this.is_sync) != 0)
}

// основной запрос, который может длиться несколько секунд
func (this *Loc) Request(val int) {

	defer func() {
		atomic.StoreInt32(this.is_sync, 0)
		fmt.Println("End")
	}()

	fmt.Println("Request...")

	this.Value = val
	time.Sleep(time.Second)
}

var list []Loc

func init() {
	list = append(list, Loc{is_sync: new(int32)})
}

func main() {
	c := time.Tick(time.Second * 2)

	for range c {
		for i := 0; i < len(list); i++ {
			if list[i].IsSync() { // если ещё не пришло время, то пропускаем
				continue
			}
			atomic.StoreInt32(list[i].is_sync, 1) //флаг нужно выставить прямо здесь, а не дожидаясь исполнения рутины
			go list[i].Request(i)
		}
	}
}

Answer 2

[Oleg Shevelev]

Вам стоит посмотреть на подход с другой стороны. Для чего вам цикл? Сделайте его в каждой горутине Request(). По одной горутине на каждый объект Loc. Для управления (стартом/стопом между итерациями) используйте канал.
До остального дойдёте сами, либо не дойдёте - как знать:)

Comments

[Павел]

Рассматривал различные варианты уже. Ваше решение с размножением горутин считаю неудачным. Сейчас поток живёт ровно столько, сколько это необходимо.
С каналами тоже экспериментировал. Вариант получился рабочим, но трудным для понимания, а это не путь Го. Кроме этого хочется всё-таки понять, почему ругается. Ведь, по логике, если добавить блокировку на время чтения, должно же вроде работать...

[Олег Цилюрик]

milast:

а это не путь Го.

Как раз использование высокоуровневого механизма каналов (синхронных или асинхронных) - это и есть "путь Го".
А вот использование низкоуровневых примитивов синхронизации из "sync" - вот это как-раз "не путь Го", о чём они не раз напоминают в документации и комменариях.

Кроме того, go-рутины - это никак не потоки (операционной системы) как вы их называете, т.е. не ветви вытесняющей многозадачности ... и здесь могут быть тонкости, если "в лоб" переносить низкоуровневую модель критических областей.