Fixing CompositeTraceWriter concurrency issue

Author: mathewc

src/WebJobs.Script/Diagnostics/CompositeTraceWriter.cs

@@ -3,22 +3,29 @@
 
 using System;
 using System.Collections.Generic;
+using System.Collections.ObjectModel;
 using System.Diagnostics;
+using System.Linq;
 using Microsoft.Azure.WebJobs.Host;
 
 namespace Microsoft.Azure.WebJobs.Script
 {
-    // TODO: The core WebJobs SDK also defines a CompositeTraceWriter, but that is internal.
-    // We should consider exposing the core SDK CompositeTraceWriter and adopting that instead.
+    // This is a copy of the WebJobs SDK CompositeTraceWriter, but that is internal.
     public class CompositeTraceWriter : TraceWriter, IDisposable
     {
-        private readonly IEnumerable<TraceWriter> _innerTraceWriters;
+        private readonly ReadOnlyCollection<TraceWriter> _innerTraceWriters;
         private bool _disposed = false;
 
         public CompositeTraceWriter(IEnumerable<TraceWriter> traceWriters, TraceLevel level = TraceLevel.Verbose)
             : base(level)
         {
-            _innerTraceWriters = traceWriters ?? throw new ArgumentNullException("traceWriters");
+            if (traceWriters == null)
+            {
+                throw new ArgumentNullException("traceWriters");
+            }
+
+            // create a copy of the collection to ensure it isn't modified
+            _innerTraceWriters = traceWriters.ToList().AsReadOnly();
         }
 
         public override void Trace(TraceEvent traceEvent)

test/WebJobs.Script.Tests/Diagnostics/CompositeTraceWriterTests.cs

@@ -2,6 +2,7 @@
 // Licensed under the MIT License. See License.txt in the project root for license information.
 
 using System;
+using System.Collections.Generic;
 using System.Diagnostics;
 using System.Linq;
 using Microsoft.Azure.WebJobs.Host;
@@ -58,6 +59,27 @@ public void Flush_FlushesInternalWriters()
             Assert.True(traceWriter.Flushed);
         }
 
+        [Fact]
+        public void Constructor_CreatesCopyOfCollection()
+        {
+            var t1 = new TestTraceWriter(TraceLevel.Verbose);
+            var t2 = new TestTraceWriter(TraceLevel.Verbose);
+            List<TraceWriter> traceWriters = new List<TraceWriter> { t1, t2 };
+            var traceWriter = new CompositeTraceWriter(traceWriters);
+
+            traceWriter.Info("Test");
+            Assert.Equal(1, t1.GetTraces().Count);
+            Assert.Equal(1, t2.GetTraces().Count);
+
+            var t3 = new TestTraceWriter(TraceLevel.Verbose);
+            traceWriters.Add(t3);
+
+            traceWriter.Info("Test");
+            Assert.Equal(2, t1.GetTraces().Count);
+            Assert.Equal(2, t2.GetTraces().Count);
+            Assert.Equal(0, t3.GetTraces().Count);
+        }
+
         [Fact]
         public void Dispose_Disposes_If_IDisposable()
         {