Merge pull request #1294 from NLog/thread-safe-scanProperties2

Fix thread-safe issue ScanProperties
NLog · Mar 7, 2016 · 13186bc · 13186bc
2 parents ccf902c + d5f26cb
commit 13186bc
Showing 1 changed file with 42 additions and 14 deletions.
diff --git a/src/NLog/Internal/ObjectGraphScanner.cs b/src/NLog/Internal/ObjectGraphScanner.cs
@@ -33,6 +33,7 @@
 
 namespace NLog.Internal
 {
+    using System;
     using System.Collections;
     using System.Collections.Generic;
     using System.Linq;
@@ -119,11 +120,12 @@ private static void ScanProperties<T>(List<T> result, object o, int level, HashS
                     continue;
                 }
 
+                List<object> elements;
                 var list = value as IList;
                 if (list != null)
                 {
                     //try first icollection for syncroot
-                    List<object> elements;
+
                     lock (list.SyncRoot)
                     {
                         elements = new List<object>(list.Count);
@@ -134,31 +136,57 @@ private static void ScanProperties<T>(List<T> result, object o, int level, HashS
                             elements.Add(item);
                         }
                     }
-                    foreach (object element in elements)
-                    {
-                        ScanProperties(result, element, level + 1, visitedObjects);
-                    }
+                    ScanPropertiesList(result, elements, level + 1, visitedObjects);
                 }
+
                 else
                 {
-                    var enumerable = value as IEnumerable;
-                    if (enumerable != null)
-                    {
-                        //new list to prevent: Collection was modified after the enumerator was instantiated.
+                    var collection = value as ICollection;
 
-                        var elements = new List<object>(enumerable.Cast<object>());
-
-                        foreach (object element in elements)
+                    if (collection != null)
+                    {
+                        object[] elementsArray = new object[collection.Count];
+                        lock (collection.SyncRoot)
                         {
-                            ScanProperties(result, element, level + 1, visitedObjects);
+                            collection.CopyTo(elementsArray, 0);
                         }
+                        ScanPropertiesList(result, elementsArray, level + 1, visitedObjects);
+
                     }
                     else
                     {
-                        ScanProperties(result, value, level + 1, visitedObjects);
+
+                        var enumerable = value as IEnumerable;
+                        if (enumerable != null)
+                        {
+                            //new list to prevent: Collection was modified after the enumerator was instantiated.
+                            //note .Cast is tread-unsafe! But at least it isn't a ICollection / IList
+                            try
+                            {
+                                elements = new List<object>(enumerable.Cast<object>());
+                            }
+                            catch (InvalidOperationException)
+                            {
+                                //retry once
+                                elements = new List<object>(enumerable.Cast<object>());
+                            }
+                            ScanPropertiesList(result, elements, level + 1, visitedObjects);
+                        }
+                        else
+                        {
+                            ScanProperties(result, value, level + 1, visitedObjects);
+                        }
                     }
                 }
             }
         }
+
+        private static void ScanPropertiesList<T>(List<T> result, IEnumerable<object> elements, int level, HashSet<object> visitedObjects) where T : class
+        {
+            foreach (object element in elements)
+            {
+                ScanProperties(result, element, level, visitedObjects);
+            }
+        }
     }
 }