From 9732d67866f164c43f459b4aed05d6b9b338dd50 Mon Sep 17 00:00:00 2001 From: Joel Hendrix Date: Tue, 20 Feb 2024 11:13:27 -0800 Subject: [PATCH] Fix potential race between NullValue and IsNullValue (#22410) Note that since IsNullValue is on a hot path, it must never block in the common case. --- sdk/azcore/CHANGELOG.md | 2 ++ sdk/azcore/core.go | 21 ++++++++++++++++++++- 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/sdk/azcore/CHANGELOG.md b/sdk/azcore/CHANGELOG.md index 28297352c208..0fa6d67a614b 100644 --- a/sdk/azcore/CHANGELOG.md +++ b/sdk/azcore/CHANGELOG.md @@ -8,6 +8,8 @@ ### Bugs Fixed +* Fixed a potential race condition between `NullValue` and `IsNullValue`. + ### Other Changes ## 1.9.2 (2024-02-06) diff --git a/sdk/azcore/core.go b/sdk/azcore/core.go index 8eef8633a7e8..9d1c2f0c0537 100644 --- a/sdk/azcore/core.go +++ b/sdk/azcore/core.go @@ -8,6 +8,7 @@ package azcore import ( "reflect" + "sync" "github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/exported" "github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/shared" @@ -41,13 +42,28 @@ func NewSASCredential(sas string) *SASCredential { } // holds sentinel values used to send nulls -var nullables map[reflect.Type]interface{} = map[reflect.Type]interface{}{} +var nullables map[reflect.Type]any = map[reflect.Type]any{} +var nullablesMu sync.RWMutex // NullValue is used to send an explicit 'null' within a request. // This is typically used in JSON-MERGE-PATCH operations to delete a value. func NullValue[T any]() T { t := shared.TypeOfT[T]() + + nullablesMu.RLock() v, found := nullables[t] + nullablesMu.RUnlock() + + if found { + // return the sentinel object + return v.(T) + } + + // promote to exclusive lock and check again (double-checked locking pattern) + nullablesMu.Lock() + defer nullablesMu.Unlock() + v, found = nullables[t] + if !found { var o reflect.Value if k := t.Kind(); k == reflect.Map { @@ -72,6 +88,9 @@ func NullValue[T any]() T { func IsNullValue[T any](v T) bool { // see if our map has a sentinel object for this *T t := reflect.TypeOf(v) + nullablesMu.RLock() + defer nullablesMu.RUnlock() + if o, found := nullables[t]; found { o1 := reflect.ValueOf(o) v1 := reflect.ValueOf(v)