nilが欲しいからと言ってむやみにポインタを使うとひどい目にあう話

スマートフォンアプリ側の実装担当として少し関わっているあるβ版のサービスで、奇妙だが比較的クリティカルな問題が発生していた。

それもメッセージが関係ない人間に届くというものであった。

なんとなくその問題を担当することになったが、常に状況が再現するわけではなく、他の仕事もあったので、問題への見当を付けられないまましばらく過ごしてしまった。

そのメッセージを送信する部分はサーバサイドのコードで、Goで書かれていた。

そのコードのエッセンスを抽出すると、以下のようになる。

package main

import (
    "fmt"
    "sync"
)

type User struct {
    Id   int
    Name string
}

type Mail struct {
    Title string
    UserId *int
}

func sendMail(mail Mail, wg *sync.WaitGroup) {
    fmt.Printf("Title=%s, UserId=%d\n", mail.Title, *mail.UserId)
    wg.Done()
}

func main() {
    users := []User{
        User{Id: 1, Name: "AAA"},
        User{Id: 2, Name: "BBB"},
    }

    wg := &sync.WaitGroup {}
    for _, user := range users {
        mail := Mail{
            Title: fmt.Sprintf("%s, check out our season offerings!", user.Name),
            UserId: &user.Id,
        }
        wg.Add(1)
        go sendMail(mail, wg)
    }
    wg.Wait()
}

The Go Playground

sendMail() に渡される引数であるところの Mail の値はループの中で都度作られているので一見よさそうに思えるのだが、実行してみると、時として次のような結果になるのだ。

Title=BBB, check out our season offerings!, UserId=2
Title=AAA, check out our season offerings!, UserId=2

(Goroutineのyieldするタイミングによって結果が変わるので、期待値通りになることもある。)

もちろん、期待値は

Title=BBB, check out our season offerings!, UserId=2
Title=AAA, check out our season offerings!, UserId=1

のように、タイトルに含まれるユーザ名とユーザのidが一致することである。

あな恐ろしや…!!!

原因は、 Mail の UserId として、ループ外のスコープで定義された user の Id メンバへのポインタが与えられ続けるためだ。

「ループ外」と書くと、「userはfor文より外のスコープで扱えないわけだからループの中だろ」というツッコミが入りそうだが、実際のスコープの内実はこうだ。

for文の外側のスコープ
- for文の宣言のスコープ (for ... := ... の部分)
  - for文の内側のスコープ ({ } の中)

この中間のスコープはブレース (中括弧) で囲まれていない範囲なので意識しづらいのだ。

さて、このコードは取り急ぎ以下のように書き換えればよい。

package main

import (
    "fmt"
    "sync"
)

type User struct {
    Id   int
    Name string
}

type Mail struct {
    Title string
    UserId *int
}

func sendMail(mail Mail, wg *sync.WaitGroup) {
    fmt.Printf("Title=%s, UserId=%d\n", mail.Title, *mail.UserId)
    wg.Done()
}

func main() {
    users := []User{
        User{Id: 1, Name: "AAA"},
        User{Id: 2, Name: "BBB"},
    }

    wg := &sync.WaitGroup {}
    for _, user := range users {
        userId = user.Id
        mail := Mail{
            Title: fmt.Sprintf("%s, check out our season offerings!", user.Name),
            UserId:  &userId,
        }
        wg.Add(1)
        go sendMail(mail, wg)
    }
    wg.Wait()
}

The Go Playground

書き換えればよい、とは書いたものの、こういうコードにすべきじゃないというのが正直なところだろうか。