From 029d151f0fd1973d2022c0bba4e7f7d705f66916 Mon Sep 17 00:00:00 2001 From: Steven Clark Date: Tue, 14 May 2024 11:06:13 -0400 Subject: [PATCH] Address a data race setting seal wrapper health attributes (#27014) * Address a data race setting seal wrapper health attributes - When updating a seal wrapper's lastHealthCheck in a failure scenario we would read the lastSeenHealthy value in order to not update it outside of a locked context. This lead to the data race. - Merge the SetHealthy and setHealthy functions into one, in order to combine the logic and locking in a single function * Add cl --- changelog/27014.txt | 3 +++ vault/seal/seal_wrapper.go | 39 ++++++++++++++++---------------------- 2 files changed, 19 insertions(+), 23 deletions(-) create mode 100644 changelog/27014.txt diff --git a/changelog/27014.txt b/changelog/27014.txt new file mode 100644 index 0000000000..94f6ebbe07 --- /dev/null +++ b/changelog/27014.txt @@ -0,0 +1,3 @@ +```release-note:bug +core: Address a data race updating a seal's last seen healthy time attribute +``` diff --git a/vault/seal/seal_wrapper.go b/vault/seal/seal_wrapper.go index 421024c7e8..48a77ff394 100644 --- a/vault/seal/seal_wrapper.go +++ b/vault/seal/seal_wrapper.go @@ -51,29 +51,32 @@ type SealWrapper struct { func NewSealWrapper(wrapper wrapping.Wrapper, priority int, name string, sealConfigType string, disabled bool, configured bool) *SealWrapper { ret := &SealWrapper{ - Wrapper: wrapper, - Priority: priority, - Name: name, - SealConfigType: sealConfigType, - Disabled: disabled, - Configured: configured, + Wrapper: wrapper, + Priority: priority, + Name: name, + SealConfigType: sealConfigType, + Disabled: disabled, + Configured: configured, + lastSeenHealthy: time.Now(), + healthy: false, } if configured { - setHealth(ret, true, time.Now(), ret.lastHealthCheck) - } else { - setHealth(ret, false, time.Now(), ret.lastHealthCheck) + ret.healthy = true } return ret } func (sw *SealWrapper) SetHealthy(healthy bool, checkTime time.Time) { + sw.hcLock.Lock() + defer sw.hcLock.Unlock() + + sw.healthy = healthy + sw.lastHealthCheck = checkTime + if healthy { - setHealth(sw, true, checkTime, checkTime) - } else { - // do not update lastSeenHealthy - setHealth(sw, false, sw.lastHealthCheck, checkTime) + sw.lastSeenHealthy = checkTime } } @@ -134,13 +137,3 @@ func getHealth(sw *SealWrapper) (healthy bool, lastSeenHealthy time.Time, lastHe return sw.healthy, sw.lastSeenHealthy, sw.lastHealthCheck } - -// setHealth is the only function allowed to mutate the health fields -func setHealth(sw *SealWrapper, healthy bool, lastSeenHealthy, lastHealthCheck time.Time) { - sw.hcLock.Lock() - defer sw.hcLock.Unlock() - - sw.healthy = healthy - sw.lastSeenHealthy = lastSeenHealthy - sw.lastHealthCheck = lastHealthCheck -}