Go Wiki: コードレビュー: Goの並行処理

このページは、Goコードレビューコメントリストへの追加です。このリストの目的は、Goコードをレビューする際に並行処理に関連するバグを見つけるのを支援することです。

また、これらの並行処理の落とし穴をすべて認識していることを確認するために、このリストを一度だけ読んで記憶を新たにすることもできます。

⚠️ このページはコミュニティによって作成および保守されています。議論の余地があり、誤解を招く可能性のある、または不正確な情報が含まれています。

不十分な同期と競合状態

テスト

スケーラビリティ

時間

不十分な同期と競合状態

# RC.1. **HTTPハンドラ関数は、複数のゴルーチンから同時に呼び出しても安全ですか？** HTTPハンドラは、通常、プロジェクトコードのどこからも明示的に呼び出されることはなく、HTTPサーバーの内部からのみ呼び出されるため、スレッドセーフである必要があることを見落とすのは簡単です。

# RC.2. **ミューテックスによって保護されていないフィールドまたは変数アクセス**があり、フィールドまたは変数がプリミティブ型または明示的にスレッドセーフではない型（atomic.Valueなど）であり、このフィールドが並行ゴルーチンから更新される可能性はありますか？ 非アトミックなハードウェア書き込みと潜在的なメモリ可視性の問題のため、プリミティブ変数への読み取りの同期をスキップすることは安全ではありません。

典型的なデータ競合：プリミティブな保護されていない変数も参照してください。

# RC.3. **スレッドセーフな型のメソッドは、保護された構造体へのポインタを返しませんか？** これは、前の項目で説明した保護されていないアクセス問題につながる微妙なバグです。 例

type Counters struct {
    mu   sync.Mutex
    vals map[Key]*Counter
}

func (c *Counters) Add(k Key, amount int) {
    c.mu.Lock()
    defer c.mu.Unlock()
    count, ok := c.vals[k]
    if !ok {
        count = &Counter{sum: 0, num: 0}
        c.vals[k] = count
    }
    count.sum += amount
    count.n += 1
}

func (c *Counters) GetCounter(k Key) *Counter {
    c.mu.Lock()
    defer c.mu.Unlock()
    return c.vals[k] // BUG! Returns a pointer to the structure which must be protected
}

考えられる解決策の1つは、GetCounter() で構造体へのポインタではなくコピーを返すことです。

type Counters struct {
    mu   sync.Mutex
    vals map[Key]Counter // Note that now we are storing the Counters directly, not pointers.
}

...

func (c *Counters) GetCounter(k Key) Counter {
    c.mu.Lock()
    defer c.mu.Unlock()
    return c.vals[k]
}

# RC.4. sync.Map を更新できるゴルーチンが複数ある場合、**以前の m.Load() 呼び出しの成功に応じて m.Store() または m.Delete() を呼び出しませんか？** つまり、次のコードは競合状態です。

var m sync.Map

// Can be called concurrently from multiple goroutines
func DoSomething(k Key, v Value) {
    existing, ok := m.Load(k)
    if !ok {
        m.Store(k, v) // RACE CONDITION - two goroutines can execute this in parallel
        ... some other logic, assuming the value in `k` is now `v` in the map
    }
    ...
}

このような競合状態は、場合によっては良性の場合があります。たとえば、Load() と Store() の呼び出しの間のロジックは、マップにキャッシュされる値を計算し、この計算は常に同じ結果を返し、副作用がありません。

⚠️ **潜在的に誤解を招く情報**。「競合状態」は、この例のように、良性である可能性のあるロジックエラーを指す場合があります。しかし、このフレーズは、決して良性ではないメモリモデルの違反を指すためにも一般的に使用されます。

競合状態が良性でない場合は、メソッド sync.Map.LoadOrStore() および LoadAndDelete() を使用して修正してください。

スケーラビリティ

# Sc.1. **チャネルが容量ゼロで作成されているのは意図的なものですか？** （例：make(chan *Foo)）容量ゼロのチャネルにメッセージを送信するゴルーチンは、別のゴルーチンがこのメッセージを受信するまでブロックされます。 make() 呼び出しで容量を省略すると、コードのスケーラビリティが制限される可能性のある単なるミスである可能性があり、単体テストではこのようなバグが見つからない可能性があります。

⚠️ **誤解を招く情報**。バッファ付きチャネルは、バッファなしチャネルと比較して、本質的に「スケーラビリティ」を向上させるわけではありません。ただし、バッファ付きチャネルは、バッファなしチャネルではすぐに明らかになるデッドロックやその他の基本的な設計エラーを簡単に隠すことができます。

# Sc.2. RWMutex でのロックは、プレーンな sync.Mutex と比較して追加のオーバーヘッドが発生し、さらに、Goの RWMutex の現在の実装にはスケーラビリティの問題がある可能性があります。ケースが非常に明確でない限り（たとえば、それぞれ数百ミリ秒以上続く多くの読み取り専用操作を同期するために使用される RWMutex と、排他ロックを必要とする書き込みがめったに発生しない場合など）、**RWMutex が実際にパフォーマンスの向上に役立つことを証明するベンチマークが必要です。** RWMutex が明らかに有害である典型的な例は、構造体内の変数の単純な保護です。

type Box struct {
    mu sync.RWMutex // DON'T DO THIS -- use a simple Mutex instead.
    x  int
}

func (b *Box) Get() int {
    b.mu.RLock()
    defer b.mu.RUnlock()
    return b.x
}

func (b *Box) Set(x int) {
    b.mu.Lock()
    defer b.mu.Unlock()
    b.x = x
}

時間

# Tm.1. **time.Ticker は defer tick.Stop() を使用して停止されていますか？** ループでティッカーを使用する関数が戻るときにティッカーを停止しないと、メモリリークが発生します。

# Tm.2. **time.Time 構造体は、== ではなく Equal() メソッドを使用して比較されていますか？** time.Timeのドキュメントを引用します。

Goの == 演算子は、タイムインスタントだけでなく、場所と単調クロックの読み取り値も比較することに注意してください。したがって、Time 値は、すべての値に同じ場所が設定されていること、および単調クロックの読み取り値が t = t.Round(0) を設定することによって削除されていることを最初に保証せずに、マップまたはデータベースキーとして使用しないでください。これは、UTC() または Local() メソッドを使用することで実現できます。一般に、t.Equal(u) は利用可能な最も正確な比較を使用し、引数の1つだけに単調クロックの読み取り値がある場合を正しく処理するため、t == u よりも t.Equal(u) を優先します。

# Tm.3. **time.Since(t) を呼び出す前に、単調増加コンポーネントは t から*削除されていませんか？** これは、前の項目の結果です。 time.Since() 関数に渡す前に、time.Time 構造体から単調増加コンポーネントが削除されている場合（UTC()、Local()、In()、Round()、Truncate()、または AddDate() を呼び出すことによって）、**time.Since() の結果は負になる可能性があります**。これは、開始時刻が最初に取得された瞬間と time.Since() が呼び出された瞬間の間に、システム時刻がNTPを介して同期された場合など、非常にまれな場合に発生します。単調増加コンポーネントが削除*されていない*場合、time.Since() は常に正の期間を返します。

# Tm.4. **t.Before(u) を介して*システム時刻*を比較する場合、引数から単調増加コンポーネントを削除しますか？** （例：u.Round(0) を介して）これは、Tm.2に関連する別のポイントです。場合によっては、2つの time.Time 構造体を、それらに格納されているシステム時刻のみで比較する必要があります。これは、これらの Time 構造体の1つをディスクに保存したり、ネットワーク経由で送信したりする前に行う必要がある場合があります。たとえば、テレメトリメトリックを समय के साथ समय-समय पर किसी दूरस्थ सिस्टम पर भेजने वाले किसी प्रकार के टेलीमेट्री एजेंट की कल्पना करें

var latestSentTime time.Time

func pushMetricPeriodically(ctx context.Context) {
    t := time.NewTicker(time.Second)
    defer t.Stop()
    for {
        select {
        case <-ctx.Done: return
        case <-t.C:
            newTime := time.Now().Round(0) // Strip monotonic component to compare system time only
            // Check that the new time is later to avoid messing up the telemetry if the system time
            // is set backwards on an NTP sync.
            if latestSentTime.Before(newTime) {
                sendOverNetwork(NewDataPoint(newTime, metric()))
                latestSentTime = newTime
            }
        }
    }
}

このコードは、Round(0) を呼び出さずに、つまり単調増加コンポーネントを削除せずに間違っています。

読書リスト

Goコードレビューコメント: Goコードをレビューするためのチェックリスト。並行処理に限定されません。

Goの並行処理

並行性（Go言語特有ではない）

このコンテンツは Go Wiki の一部です。

go.devは、GoogleのCookieを使用してサービスの提供と品質の向上、トラフィックの分析を行っています。 詳細はこちら