From f9d6098dc0db885f4edae9c454e92ff981db369f Mon Sep 17 00:00:00 2001 From: semick-dev Date: Thu, 2 May 2024 23:40:34 -0700 Subject: [PATCH] add some judicious locking --- .../Common/ModifiableRecordSession.cs | 19 ++-- .../Common/SanitizerDictionary.cs | 91 +++++++++++++------ 2 files changed, 74 insertions(+), 36 deletions(-) diff --git a/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Common/ModifiableRecordSession.cs b/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Common/ModifiableRecordSession.cs index 96d9952a318..dc7eff129af 100644 --- a/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Common/ModifiableRecordSession.cs +++ b/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Common/ModifiableRecordSession.cs @@ -35,6 +35,8 @@ public ModifiableRecordSession(RecordSession session, SanitizerDictionary saniti public List AdditionalTransforms { get; } = new List(); + public readonly object SanitizerLock = new object(); + public List AppliedSanitizers { get; set; } = new List(); public List ForRemoval { get; } = new List(); @@ -44,13 +46,16 @@ public ModifiableRecordSession(RecordSession session, SanitizerDictionary saniti public void ResetExtensions(SanitizerDictionary sanitizerDictionary) { - AdditionalTransforms.Clear(); - AppliedSanitizers = new List(); - AppliedSanitizers.AddRange(sanitizerDictionary.SessionSanitizers); - ForRemoval.Clear(); - - CustomMatcher = null; - Client = null; + lock (SanitizerLock) + { + AdditionalTransforms.Clear(); + AppliedSanitizers = new List(); + AppliedSanitizers.AddRange(sanitizerDictionary.SessionSanitizers); + ForRemoval.Clear(); + + CustomMatcher = null; + Client = null; + } } } } diff --git a/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Common/SanitizerDictionary.cs b/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Common/SanitizerDictionary.cs index 964f1d4a701..80c31588c16 100644 --- a/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Common/SanitizerDictionary.cs +++ b/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Common/SanitizerDictionary.cs @@ -43,6 +43,8 @@ public class SanitizerDictionary // apply only the sanitizers that have been registered at the global level public List SessionSanitizers = new List(); + public readonly object SessionSanitizerLock = new object(); + public SanitizerDictionary() { ResetSessionSanitizers(); } @@ -693,20 +695,23 @@ public SanitizerDictionary() { /// public void ResetSessionSanitizers() { - var expectedSanitizers = DefaultSanitizerList; - - for (int i = 0; i < expectedSanitizers.Count; i++) + lock (SessionSanitizerLock) { - var id = expectedSanitizers[i].Id; - var sanitizer = expectedSanitizers[i].Sanitizer; + var expectedSanitizers = DefaultSanitizerList; - if (!Sanitizers.ContainsKey(id)) + for (int i = 0; i < expectedSanitizers.Count; i++) { - _register(sanitizer, id); + var id = expectedSanitizers[i].Id; + var sanitizer = expectedSanitizers[i].Sanitizer; + + if (!Sanitizers.ContainsKey(id)) + { + _register(sanitizer, id); + } } - } - SessionSanitizers = DefaultSanitizerList.Select(x => x.Id).ToList(); + SessionSanitizers = DefaultSanitizerList.Select(x => x.Id).ToList(); + } } /// @@ -736,11 +741,19 @@ public List GetSanitizers() public List GetRegisteredSanitizers(ModifiableRecordSession session) { var sanitizers = new List(); - foreach (var id in session.AppliedSanitizers) + + lock (session.SanitizerLock) { - if (Sanitizers.TryGetValue(id, out RegisteredSanitizer sanitizer)) + foreach (var id in session.AppliedSanitizers) { - sanitizers.Add(sanitizer); + if (Sanitizers.TryGetValue(id, out RegisteredSanitizer sanitizer)) + { + sanitizers.Add(sanitizer); + } + else + { + DebugLogger.LogError($"Failed to get a sanitizer with id {id}"); + } } } @@ -754,11 +767,14 @@ public List GetRegisteredSanitizers(ModifiableRecordSession public List GetRegisteredSanitizers() { var sanitizers = new List(); - foreach (var id in SessionSanitizers) + lock (SessionSanitizerLock) { - if (Sanitizers.TryGetValue(id, out RegisteredSanitizer sanitizer)) + foreach (var id in SessionSanitizers) { - sanitizers.Add(sanitizer); + if (Sanitizers.TryGetValue(id, out RegisteredSanitizer sanitizer)) + { + sanitizers.Add(sanitizer); + } } } @@ -788,10 +804,13 @@ public string Register(RecordedTestSanitizer sanitizer) { var strCurrent = IdFactory.GetNextId().ToString(); - if (_register(sanitizer, strCurrent)) + lock (SessionSanitizerLock) { - SessionSanitizers.Add(strCurrent); - return strCurrent; + if (_register(sanitizer, strCurrent)) + { + SessionSanitizers.Add(strCurrent); + return strCurrent; + } } throw new HttpException(System.Net.HttpStatusCode.InternalServerError, $"Unable to register global sanitizer id \"{strCurrent}\" with value '{JsonSerializer.Serialize(sanitizer)}'"); } @@ -806,12 +825,17 @@ public string Register(RecordedTestSanitizer sanitizer) public string Register(ModifiableRecordSession session, RecordedTestSanitizer sanitizer) { var strCurrent = IdFactory.GetNextId().ToString(); - if (_register(sanitizer, strCurrent)) + + lock (session.SanitizerLock) { - session.AppliedSanitizers.Add(strCurrent); - session.ForRemoval.Add(strCurrent); - return strCurrent; + if (_register(sanitizer, strCurrent)) + { + session.AppliedSanitizers.Add(strCurrent); + session.ForRemoval.Add(strCurrent); + + return strCurrent; + } } return string.Empty; @@ -825,10 +849,13 @@ public string Register(ModifiableRecordSession session, RecordedTestSanitizer sa /// public string Unregister(string sanitizerId) { - if (SessionSanitizers.Contains(sanitizerId)) + lock (SessionSanitizerLock) { - SessionSanitizers.Remove(sanitizerId); - return sanitizerId; + if (SessionSanitizers.Contains(sanitizerId)) + { + SessionSanitizers.Remove(sanitizerId); + return sanitizerId; + } } throw new HttpException(System.Net.HttpStatusCode.BadRequest, $"The requested sanitizer for removal \"{sanitizerId}\" is not active at the session level."); @@ -843,10 +870,13 @@ public string Unregister(string sanitizerId) /// public string Unregister(string sanitizerId, ModifiableRecordSession session) { - if (session.AppliedSanitizers.Contains(sanitizerId)) + lock (session.SanitizerLock) { - session.AppliedSanitizers.Remove(sanitizerId); - return sanitizerId; + if (session.AppliedSanitizers.Contains(sanitizerId)) + { + session.AppliedSanitizers.Remove(sanitizerId); + return sanitizerId; + } } throw new HttpException(System.Net.HttpStatusCode.BadRequest, $"The requested sanitizer for removal \"{sanitizerId}\" is not active on recording/playback with id \"{session.SessionId}\"."); @@ -869,7 +899,10 @@ public void Cleanup(ModifiableRecordSession session) /// public void Clear() { - SessionSanitizers.Clear(); + lock (SessionSanitizerLock) + { + SessionSanitizers.Clear(); + } } } }