From e17740bb2d0b184cb1a43dec1cbab31f04121237 Mon Sep 17 00:00:00 2001 From: Nathan Willoughby Date: Tue, 5 Mar 2024 16:54:15 +1000 Subject: [PATCH] YamlDotNot no longer appears to be thread-safe --- src/KubernetesClient/KubernetesClient.csproj | 2 + src/KubernetesClient/KubernetesYaml.cs | 29 ++++++-- .../KubernetesClientConfigurationTests.cs | 67 ++++++++++++++++++- 3 files changed, 91 insertions(+), 7 deletions(-) diff --git a/src/KubernetesClient/KubernetesClient.csproj b/src/KubernetesClient/KubernetesClient.csproj index e32d6d70b..4a09e0b49 100644 --- a/src/KubernetesClient/KubernetesClient.csproj +++ b/src/KubernetesClient/KubernetesClient.csproj @@ -14,6 +14,8 @@ + + diff --git a/src/KubernetesClient/KubernetesYaml.cs b/src/KubernetesClient/KubernetesYaml.cs index b40ed8d44..9b7e20776 100644 --- a/src/KubernetesClient/KubernetesYaml.cs +++ b/src/KubernetesClient/KubernetesYaml.cs @@ -12,6 +12,27 @@ namespace k8s /// public static class KubernetesYaml { + //private static DeserializerBuilder GetCommonDeserializerBuilder() => + // new DeserializerBuilder() + // .WithNamingConvention(CamelCaseNamingConvention.Instance) + // .WithTypeConverter(new IntOrStringYamlConverter()) + // .WithTypeConverter(new ByteArrayStringYamlConverter()) + // .WithTypeConverter(new ResourceQuantityYamlConverter()) + // .WithAttemptingUnquotedStringTypeDeserialization() + // .WithOverridesFromJsonPropertyAttributes(); + + //private static IDeserializer GetStrictDeserializer() => + // GetCommonDeserializerBuilder() + // .WithDuplicateKeyChecking() + // .Build(); + + //private static IDeserializer GetDeserializer() => + // GetCommonDeserializerBuilder() + // .IgnoreUnmatchedProperties() + // .Build(); + + //private static IDeserializer GetDeserializer(bool strict) => strict ? GetStrictDeserializer() : GetDeserializer(); + private static DeserializerBuilder CommonDeserializerBuilder => new DeserializerBuilder() .WithNamingConvention(CamelCaseNamingConvention.Instance) @@ -23,12 +44,12 @@ public static class KubernetesYaml private static readonly IDeserializer StrictDeserializer = CommonDeserializerBuilder - .WithDuplicateKeyChecking() - .Build(); + .WithDuplicateKeyChecking() + .Build(); private static readonly IDeserializer Deserializer = CommonDeserializerBuilder - .IgnoreUnmatchedProperties() - .Build(); + .IgnoreUnmatchedProperties() + .Build(); private static IDeserializer GetDeserializer(bool strict) => strict ? StrictDeserializer : Deserializer; private static readonly IValueSerializer Serializer = diff --git a/tests/KubernetesClient.Tests/KubernetesClientConfigurationTests.cs b/tests/KubernetesClient.Tests/KubernetesClientConfigurationTests.cs index a1239c9e9..2517493e8 100644 --- a/tests/KubernetesClient.Tests/KubernetesClientConfigurationTests.cs +++ b/tests/KubernetesClient.Tests/KubernetesClientConfigurationTests.cs @@ -1,7 +1,5 @@ -using k8s.Authentication; -using k8s.Exceptions; -using k8s.KubeConfigModels; using System; +using System.Collections.Concurrent; using System.Collections.Generic; using System.IO; using System.IO.Abstractions; @@ -10,6 +8,10 @@ using System.Runtime.InteropServices; using System.Threading; using System.Threading.Tasks; +using FluentAssertions; +using k8s.Authentication; +using k8s.Exceptions; +using k8s.KubeConfigModels; using Xunit; namespace k8s.Tests @@ -638,6 +640,65 @@ public void ContextPreferencesExtensionsMergeWithDuplicates() Assert.Single(cfg.Preferences); } + [Fact] + public void LoadKubeConfigShouldBeThreadSafe() + { + // This is not guaranteed to fail but it usual throws the follow exception + // - System.InvalidOperationException: Operations that change non-concurrent collections must have exclusive access. + // A concurrent update was performed on this collection and corrupted its state. The collection's state is no longer correct. + // at System.Collections.Generic.Dictionary`2.TryInsert(TKey key, TValue value, InsertionBehavior behavior) + // at System.Collections.Generic.Dictionary`2.set_Item(TKey key, TValue value) + // at YamlDotNet.Serialization.ObjectFactories.DefaultObjectFactory.GetStateMethods(Type attributeType, Type valueType) + // at YamlDotNet.Serialization.ObjectFactories.DefaultObjectFactory.ExecuteState(Type attributeType, Object value) + // at YamlDotNet.Serialization.ObjectFactories.DefaultObjectFactory.ExecuteOnDeserializing(Object value) + // at YamlDotNet.Serialization.NodeDeserializers.ObjectNodeDeserializer.Deserialize(IParser parser, Type expectedType, Func`3 nestedObjectDeserializer, Object& value) + // at YamlDotNet.Serialization.ValueDeserializers.NodeValueDeserializer.DeserializeValue(IParser parser, Type expectedType, SerializerState state, IValueDeserializer nestedObjectDeserializer)}. + + // YamlDotNet Deserializer does not seems to be thread safe, changing KubernetesYaml to not use a static serializer makes this test pass. + + var exceptions = new ConcurrentStack(); + + // Run it many times for a better failure rate + Run(exceptions); + Run(exceptions); + Run(exceptions); + Run(exceptions); + Run(exceptions); + + exceptions.Should().HaveCount(0, "No exceptions should have been recorded"); + + static void Run(ConcurrentStack exceptions) + { + var threadCount = 100; + var threads = new List(); + var control = new SemaphoreSlim(0, threadCount); + + for (var i = 0; i < threadCount; i++) + { + threads.Add(new Thread(LoadKubeConfig)); + } + + threads.ForEach(t => t.Start()); + control.Release(threadCount); + threads.ForEach(t => t.Join()); + + void LoadKubeConfig() + { + control.Wait(); + + try + { + var fileInfo = new FileInfo(Path.GetFullPath("assets/kubeconfig.yml")); + KubernetesClientConfiguration.LoadKubeConfig(new [] { fileInfo }); + } + catch (Exception e) + { + exceptions.Push(e.InnerException ?? e); + } + } + } + } + /// /// Ensures Kube config file can be loaded from within a non-default . /// The use of ensures the test is run from within a UI-like .