From c4a9560d53e5da14dd5eaf12b2863aadf04bab45 Mon Sep 17 00:00:00 2001 From: James Shubin Date: Fri, 5 Jan 2024 15:13:08 -0500 Subject: [PATCH] engine: local: Fix benign race Use the mutex in a safer manner to eliminate the benign data race. --- engine/local/local.go | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/engine/local/local.go b/engine/local/local.go index 12030c4d..e4d7e5a8 100644 --- a/engine/local/local.go +++ b/engine/local/local.go @@ -235,12 +235,18 @@ func (obj *Value) ValueWatch(ctx context.Context, key string) (chan struct{}, er // getPrefix gets the prefix dir to use, or errors if it can't make one. It // makes it on first use, and returns quickly from any future calls to it. func (obj *Value) getPrefix() (string, error) { - if obj.prefixExists { - return obj.prefix, nil - } + // NOTE: Moving this mutex to just below the first early return, would + // be a benign race, but as it turns out, it's possible that a compiler + // would see this behaviour as "undefined" and things might not work as + // intended. It could perhaps be replaced with a sync/atomic primitive + // if we wanted better performance here. obj.mutex.Lock() defer obj.mutex.Unlock() + if obj.prefixExists { // former race read + return obj.prefix, nil + } + // MkdirAll instead of Mkdir because we have no idea if the parent // local/ directory was already made yet or not. (If at all.) If path is // already a directory, MkdirAll does nothing and returns nil. (Good!) @@ -249,7 +255,7 @@ func (obj *Value) getPrefix() (string, error) { if err := os.MkdirAll(obj.prefix, 0755); err != nil { return "", err } - obj.prefixExists = true + obj.prefixExists = true // former race write return obj.prefix, nil }