Merge branch 'fix-exception-on-macos' into 'main'

Fix exception occuring sometimes on macos when Accessing UniqueList.SortedValues See merge request Sharpmake/sharpmake!587
ubisoft · Nov 29, 2024 · f4183f4 · f4183f4
2 parents 669ceb8 + 0b7e2e1
commit f4183f4
Show file tree

Hide file tree

Showing 6 changed files with 201 additions and 89 deletions.
diff --git a/Sharpmake.UnitTests/UniqueListTests.cs b/Sharpmake.UnitTests/UniqueListTests.cs
@@ -507,6 +507,9 @@ public ListContainer()
             }
             public UniqueList<string> List = new UniqueList<string>();
             public IEnumerable<string> SortedList => List.SortedValues;
+
+            public long IterationCount = 0;
+            public long NonEmptyCount = 0;
         }
 
         /// <summary>
@@ -516,27 +519,32 @@ public ListContainer()
         /// exception when multiple threads were accessing the property.
         /// </summary>
         [Test]
-        public static void MultithreadEmptyValuesSorted()
+        [TestCase("")]
+        [TestCase("Test")]
+        public static void MultithreadEmptyValuesSorted(string initialContent)
         {
             int nbrThreads = Environment.ProcessorCount;
             var container = new ListContainer();
+            if (!string.IsNullOrEmpty(initialContent))
+                container.List.Add(initialContent);
 
             int TOTAL_TEST_COUNT = 100000;
             long nbrThreadsFinished = 0;
             long nbrThreadsGate1 = 0;
             Exception taskTestException = null;
 
             // Note: Using a Barrier to synchronize all the threads at each iteration
-            using (Barrier barrier = new Barrier(nbrThreads, (b) =>
+            using (Barrier barrier = new Barrier(0, (b) =>
              {
+                 container.List.SetDirty();
                  Interlocked.Increment(ref nbrThreadsGate1);
-                 container.List.AddRange(new List<string> { }); // Adding an empty collection makes the UniqueList dirty
              }))
             {
                 ThreadPool.TaskCallback taskLambda = (object taskParams) =>
                 {
                     var listContainersTask = (ListContainer)taskParams;
-
+                    int count = 0;
+                    int nonEmptyCount = 0;
                     try
                     {
                         for (int i = 0; i < TOTAL_TEST_COUNT; ++i)
@@ -547,11 +555,15 @@ public static void MultithreadEmptyValuesSorted()
                             if (taskTestException != null)
                                 break; // Abort once we got an exception
 
-                            // Attempt to access the SortedList property from multiple threads. 
-                            // It must not create any exception!
-                            foreach (var s in container.SortedList)
+                            for (int j = 0; j < 5; ++j)
                             {
-                                Console.WriteLine(s);
+                                // Attempt to access the SortedList property from multiple threads. 
+                                // It must not create any exception!
+                                foreach (var s in container.SortedList)
+                                {
+                                    ++nonEmptyCount;
+                                }
+                                ++count;
                             }
                         }
                     }
@@ -562,8 +574,10 @@ public static void MultithreadEmptyValuesSorted()
                     }
                     finally
                     {
-                        barrier.RemoveParticipant(); // Must remove the participant to unblock all the other threads.
                         Interlocked.Increment(ref nbrThreadsFinished);
+                        Interlocked.Add(ref listContainersTask.IterationCount, count);
+                        Interlocked.Add(ref listContainersTask.IterationCount, nonEmptyCount);
+                        barrier.RemoveParticipant();
                     }
                 };
 
@@ -572,6 +586,7 @@ public static void MultithreadEmptyValuesSorted()
                 {
                     // Add 1 task per thread
                     pool.Start(nbrThreads);
+                    barrier.AddParticipants(nbrThreads);
                     for (int i = 0; i < nbrThreads; ++i)
                     {
                         pool.AddTask(taskLambda, container);
@@ -582,6 +597,7 @@ public static void MultithreadEmptyValuesSorted()
 
                     // Check the results.
                     TestContext.Out.WriteLine("nbr Finished: {0}, nbr Gate1: {1}", nbrThreadsFinished, nbrThreadsGate1);
+                    TestContext.Out.WriteLine($"IterationCount : {container.IterationCount}");
                     if (taskTestException != null)
                     {
                         throw taskTestException;

diff --git a/Sharpmake/Options.XCode.cs b/Sharpmake/Options.XCode.cs
@@ -1027,29 +1027,17 @@ public UIInterfaceOrientation_iPad(UIInterfaceOrientation value)
                        : base(value) { }
                 }
 
-                public class UISupportedInterfaceOrientations : UniqueList<UIInterfaceOrientation>
+                public class UISupportedInterfaceOrientations
                 {
+                    private UIInterfaceOrientation[] _uIInterfaceOrientations;
                     public UISupportedInterfaceOrientations(params UIInterfaceOrientation[] values)
-                        : base(EqualityComparer<UIInterfaceOrientation>.Default, values) { }
+                    {
+                        _uIInterfaceOrientations = values;
+                    }
 
                     public override string ToString()
                     {
-                        StringBuilder builder = new StringBuilder(Count * 128);
-                        bool first = true;
-                        foreach (UIInterfaceOrientation value in _hash)
-                        {
-                            if (!first)
-                                builder.Append(' ');
-                            else
-                                first = false;
-
-                            builder.Append(value.ToString());
-                        }
-
-                        if (_hash.Count > 1)
-                            return @$"""{builder.ToString()}""";
-
-                        return builder.ToString();
+                        return "\"" + string.Join(" ", _uIInterfaceOrientations) + "\"";
                     }
                 }
 

diff --git a/Sharpmake/Strings.cs b/Sharpmake/Strings.cs
@@ -13,14 +13,12 @@ namespace Sharpmake
     public class Strings : UniqueList<string>
     {
         public Strings(IEqualityComparer<string> hashComparer, IComparer<string> sortComparer)
-            : base(hashComparer)
+            : base(hashComparer, sortComparer)
         {
-            SortComparer = sortComparer;
         }
         public Strings()
-            : base(StringComparer.OrdinalIgnoreCase)
+            : base(StringComparer.OrdinalIgnoreCase, StringComparer.OrdinalIgnoreCase)
         {
-            SortComparer = StringComparer.OrdinalIgnoreCase;
         }
 
         public Strings(IEnumerable<string> other) : base(StringComparer.OrdinalIgnoreCase, other) { }