YamlDotNot no longer appears to be thread-safe

Author: nathanwoctopusdeploy

src/KubernetesClient/KubernetesClient.csproj

@@ -14,6 +14,8 @@
         <PackageReference Include="IdentityModel.OidcClient" Version="5.2.1" />
         <PackageReference Include="Fractions" Version="7.3.0" />
         <PackageReference Include="YamlDotNet" Version="15.1.0" />
+        <!--Same issue with the latest version of YamlDotNet-->
+        <!--<PackageReference Include="YamlDotNet" Version="15.1.2" />-->
     </ItemGroup>
 
     <ItemGroup>

src/KubernetesClient/KubernetesYaml.cs

@@ -12,6 +12,27 @@ namespace k8s
     /// </summary>
     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 =

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 no longer seems to be thread safe, changing KubernetesYaml to not use a static serializer makes this test pass.
+
+            var exceptions = new ConcurrentStack<Exception>();
+
+            // 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<Exception> exceptions)
+            {
+                var threadCount = 100;
+                var threads = new List<Thread>();
+                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);
+                    }
+                }
+            }
+        }
+
         /// <summary>
         ///     Ensures Kube config file can be loaded from within a non-default <see cref="SynchronizationContext"/>.
         ///     The use of <see cref="UIFactAttribute"/> ensures the test is run from within a UI-like <see cref="SynchronizationContext"/>.