Node access is done using locked Node copy #3461
Conversation
Fixes #3454 Reliably reproduced the data race before by having a fingerprinter change the nodes attributes every millisecond and syncing at the same rate. With fix, did not ever panic.
| @@ -501,7 +501,7 @@ func (c *Client) CollectAllAllocs() error { | |||
| func (c *Client) Node() *structs.Node { | |||
| c.configLock.RLock() | |||
| defer c.configLock.RUnlock() | |||
| return c.config.Node | |||
| return c.configCopy.Node | |||
| } | |||
There was a problem hiding this comment.
There are other public methods in this file that access fields from config rather than configcopy. - datacenter/region/nodeid etc. None of them are maps, but are other lurking races there as well?
There was a problem hiding this comment.
Those fields are immutable once the Client is returned, the mutable fields are the attributes and links, which surfaced via the Node() and Fingerprint() using the same Node object rather than the readonly copy.
|
I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions. |
Fixes #3454
Reliably reproduced the data race before by having a fingerprinter
change the nodes attributes every millisecond and syncing at the same
rate. With fix, did not ever panic. Further, running before and after with race
detector shows the data race is gone.