Bug: Fix race condition in os_release.go

[RNabel]

Problem

os_release.Update may be called concurrently; it in turn calls UpdateStruct which:

1. Reads c.osFilename and c.osMtime, if both values are set and match some condition the function it returns. Update assumes that c.os is set if this path is executed.
2. Acquires mutex (line 132)
3. Sets c.osFilename and c.osMtime
4. Sets c.os
5. Releases mutex

There is a race condition here: the reads in 1. are not protected by a mutex, so the following pathological interleaving will occasionally occur:
thread t1 will execute step 3., followed by
thread t2 executing step 1., returns from UpdateStruct assuming that c.os is set, execution continues in line 159, breaks out of the loop and c.os is accessed in line 173, resulting in a segfault as it is (nil,nil).

Solution

One option is to wrap accesses of c.osFilename and c.osMtime in a mutex. In my use-case, running with this patch has resolved all segfaults which previously occurred in ~1% of node_exporter executions.

It also seems plausible to address this by re-ordering steps 3. and 4. and/or to move the mutex acquire above the if statement to shield the reads with the existing mutex acquire. Let me know if you'd prefer either of these solutions.

Signed-off-by: Robin Nabel rnabel@ucdavis.edu
(referencing @discordianfish as per your Contributing guide)

[SuperQ]

Thanks

[SuperQ]

The codespell test is unrelated, fixing that in another PR.

[discordianfish]

LGTM