hashicorp / memberlist

Golang package for gossip based membership and failure detection
Mozilla Public License 2.0
3.61k stars 435 forks source link

Data race in state.go:1135 #306

Open vitalif opened 1 month ago

vitalif commented 1 month ago

Hi. I've detected the following data race in memberlist 0.5.0:

==================
WARNING: DATA RACE
Write at 0x00c000693728 by goroutine 53:
  github.com/hashicorp/memberlist.(*Memberlist).aliveNode()
      .../vendor/github.com/hashicorp/memberlist/state.go:1135 +0x2e3b
  github.com/hashicorp/memberlist.(*Memberlist).handleAlive()
      .../vendor/github.com/hashicorp/memberlist/net.go:738 +0x69a
  github.com/hashicorp/memberlist.(*Memberlist).packetHandler()
      .../vendor/github.com/hashicorp/memberlist/net.go:513 +0x1b9
  github.com/hashicorp/memberlist.newMemberlist.gowrap3()
      .../vendor/github.com/hashicorp/memberlist/memberlist.go:235 +0x33

Previous read at 0x00c000693728 by goroutine 146:
  github.com/hashicorp/memberlist.(*Node).Address()
      .../vendor/github.com/hashicorp/memberlist/state.go:58 +0x6b
...

Goroutine 53 (running) created at:
  github.com/hashicorp/memberlist.newMemberlist()
      .../vendor/github.com/hashicorp/memberlist/memberlist.go:235 +0x1aea
  github.com/hashicorp/memberlist.Create()
      .../vendor/github.com/hashicorp/memberlist/memberlist.go:246 +0x2e
...

Goroutine 146 (running) created at:
...
==================

The idea is that memberlist Members() returns a list of pointers to Memberlist::nodes array items. Then the external user's code is allowed to read them without any locking. But at the same time, they may get modified internally in Memberlist, resulting in a data race.

Not sure if it's a critical issue and how hard it would be to stumble on it in production. But it's still a race...

vitalif commented 1 month ago

May be related to #272 but I'm not sure.