Fix infinite loop in TempFileCollection if the directory does not exist

src/Common/tests/System/Collections/ICollection.NonGeneric.Tests.cs

@@ -78,6 +78,12 @@ protected virtual ICollection NonGenericICollectionFactory(int count)
         /// </summary>
         protected virtual bool ICollection_NonGeneric_SupportsSyncRoot => true;
 
+        /// <summary>
+        /// Used for ICollection_NonGeneric_SyncRoot tests. Some implementations (e.g. TempFileCollection)
+        /// return null for the SyncRoot property of an ICollection.
+        /// </summary>
+        protected virtual bool ICollection_NonGeneric_HasNullSyncRoot => false;
+
         /// <summary>
         /// Used for the ICollection_NonGeneric_SyncRootType_MatchesExcepted test. Most SyncRoots are created
         /// using System.Threading.Interlocked.CompareExchange(ref _syncRoot, new Object(), null)
@@ -135,74 +141,48 @@ public void ICollection_NonGeneric_IsSynchronized(int count)
 
         [Theory]
         [MemberData(nameof(ValidCollectionSizes))]
-        public void ICollection_NonGeneric_SyncRoot_NonNull(int count)
+        public void ICollection_NonGeneric_SyncRoot(int count)
         {
+            ICollection collection = NonGenericICollectionFactory(count);
             if (ICollection_NonGeneric_SupportsSyncRoot)
             {
-                ICollection collection = NonGenericICollectionFactory(count);
-                Assert.NotNull(collection.SyncRoot);
-            }
-        }
+                Assert.Equal(ICollection_NonGeneric_HasNullSyncRoot, collection.SyncRoot == null);
+                Assert.Same(collection.SyncRoot, collection.SyncRoot);
 
-        [Theory]
-        [MemberData(nameof(ValidCollectionSizes))]
-        public void ICollection_NonGeneric_SyncRootConsistent(int count)
-        {
-            if (ICollection_NonGeneric_SupportsSyncRoot)
+                if (!ICollection_NonGeneric_HasNullSyncRoot)
+                {
+                    Assert.IsType(ICollection_NonGeneric_SyncRootType, collection.SyncRoot);
+
+                    if (ICollection_NonGeneric_SyncRootType == collection.GetType())
+                    {
+                        // If we expect the SyncRoot to be the same type as the collection, 
+                        // the SyncRoot should be the same as the collection (e.g. HybridDictionary)
+                        Assert.Same(collection, collection.SyncRoot);
+                    }
+                    else
+                    {
+                        Assert.NotSame(collection, collection.SyncRoot);
+                    }
+                }
+            }
+            else
             {
-                ICollection collection = NonGenericICollectionFactory(count);
-                object syncRoot1 = collection.SyncRoot;
-                object syncRoot2 = collection.SyncRoot;
-                Assert.Same(syncRoot1, syncRoot2);
+                Assert.Throws<NotSupportedException>(() => collection.SyncRoot);
             }
         }
 
         [Theory]
         [MemberData(nameof(ValidCollectionSizes))]
         public void ICollection_NonGeneric_SyncRootUnique(int count)
         {
-            if (ICollection_NonGeneric_SupportsSyncRoot)
+            if (ICollection_NonGeneric_SupportsSyncRoot && !ICollection_NonGeneric_HasNullSyncRoot)
             {
                 ICollection collection1 = NonGenericICollectionFactory(count);
                 ICollection collection2 = NonGenericICollectionFactory(count);
                 Assert.NotSame(collection1.SyncRoot, collection2.SyncRoot);
             }
         }
 
-        [Theory]
-        [MemberData(nameof(ValidCollectionSizes))]
-        public void ICollection_NonGeneric_SyncRoot_MatchesExpectedType(int count)
-        {
-            if (ICollection_NonGeneric_SupportsSyncRoot)
-            {
-                ICollection collection = NonGenericICollectionFactory(count);
-
-                Assert.IsType(ICollection_NonGeneric_SyncRootType, collection.SyncRoot);
-
-                if (ICollection_NonGeneric_SyncRootType == collection.GetType())
-                {
-                    // If we expect the SyncRoot to be the same type as the collection, 
-                    // the SyncRoot should be the same as the collection (e.g. HybridDictionary)
-                    Assert.Same(collection, collection.SyncRoot);
-                }
-                else
-                {
-                    Assert.NotSame(collection, collection.SyncRoot);
-                }
-            }
-        }
-
-        [Theory]
-        [MemberData(nameof(ValidCollectionSizes))]
-        public void ICollection_NonGeneric_SyncRoot_ThrowsNotSupportedException(int count)
-        {
-            if (!ICollection_NonGeneric_SupportsSyncRoot)
-            {
-                ICollection collection = NonGenericICollectionFactory(count);
-                Assert.Throws<NotSupportedException>(() => collection.SyncRoot);
-            }
-        }
-
         #endregion
 
         #region CopyTo

src/System.CodeDom/src/System/CodeDom/Compiler/TempFiles.cs

@@ -112,17 +112,20 @@ private void EnsureTempNameCreated()
                     _basePath = Path.Combine(
                         string.IsNullOrEmpty(TempDir) ? Path.GetTempPath() : TempDir,
                         Path.GetFileNameWithoutExtension(Path.GetRandomFileName()));
-                    string full = Path.GetFullPath(_basePath);
                     tempFileName = _basePath + ".tmp";
 
                     try
                     {
                         new FileStream(tempFileName, FileMode.CreateNew, FileAccess.Write).Dispose();
                         uniqueFile = true;
                     }
-                    catch (IOException)
+                    catch (IOException ex)
                     {
                         retryCount--;
+                        if (retryCount == 0 || ex is DirectoryNotFoundException)
+                        {
+                            throw;
+                        }
                         uniqueFile = false;
                     }
                 } while (!uniqueFile);

src/System.CodeDom/tests/CodeCollections/TempFileCollectionTests.cs

@@ -0,0 +1,263 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+using System.CodeDom.Compiler;
+using System.Collections;
+using System.Collections.Generic;
+using System.Collections.Tests;
+using System.IO;
+using Xunit;
+
+namespace System.CodeDom.Tests
+{
+    public class TempFileCollectionTests : ICollection_NonGeneric_Tests
+    {
+        protected override ICollection NonGenericICollectionFactory() => new TempFileCollection();
+
+        protected override void AddToCollection(ICollection collection, int numberOfItemsToAdd)
+        {
+            Random random = new Random(numberOfItemsToAdd);
+            for (int i = 0; i < numberOfItemsToAdd; i++)
+            {
+                ((TempFileCollection)collection).AddFile(random.Next().ToString(), keepFile: true);
+            }
+        }
+
+        protected override bool NullAllowed => false;
+        protected override bool Enumerator_Current_UndefinedOperation_Throws => true;
+        protected override bool ICollection_NonGeneric_HasNullSyncRoot => true;
+
+        protected override Type ICollection_NonGeneric_CopyTo_ArrayOfEnumType_ThrowType => typeof(InvalidCastException);
+        protected override Type ICollection_NonGeneric_CopyTo_ArrayOfIncorrectValueType_ThrowType => typeof(InvalidCastException);
+        protected override Type ICollection_NonGeneric_CopyTo_ArrayOfIncorrectReferenceType_ThrowType => typeof(InvalidCastException);
+
+        public override void ICollection_NonGeneric_CopyTo_NonZeroLowerBound(int count)
+        {
+            ICollection collection = NonGenericICollectionFactory(count);
+            Array arr = Array.CreateInstance(typeof(object), new int[1] { count }, new int[1] { 2 });
+
+            if (count == 0)
+            {
+                collection.CopyTo(arr, 0);
+            }
+            else
+            {
+                Assert.Throws<IndexOutOfRangeException>(() => collection.CopyTo(arr, 0));
+            }
+        }
+
+        [Fact]
+        public void Ctor_Empty()
+        {
+            var collection = new TempFileCollection();
+            Assert.Equal(0, collection.Count);
+            Assert.Empty(collection.TempDir);
+            Assert.False(collection.KeepFiles);
+        }
+
+        [Theory]
+        [InlineData(null)]
+        [InlineData("")]
+        [InlineData("TempDir")]
+        public void Ctor_String(string tempDir)
+        {
+            var collection = new TempFileCollection(tempDir);
+            Assert.Equal(0, collection.Count);
+            Assert.Equal(tempDir ?? string.Empty, collection.TempDir);
+            Assert.False(collection.KeepFiles);
+        }
+
+        [Theory]
+        [InlineData(null, false)]
+        [InlineData("", true)]
+        [InlineData("TempDir", false)]
+        public void Ctor_String_Bool(string tempDir, bool keepFiles)
+        {
+            var collection = new TempFileCollection(tempDir, keepFiles);
+            Assert.Equal(0, collection.Count);
+            Assert.Equal(tempDir ?? string.Empty, collection.TempDir);
+            Assert.Equal(keepFiles, collection.KeepFiles);
+        }
+
+        public static IEnumerable<object[]> BasePath_TestData()
+        {
+            yield return new object[] { "NoSuchDirectory" };
+            yield return new object[] { TempDirectory() };
+        }
+
+        [Theory]
+        [MemberData(nameof(BasePath_TestData))]
+        public void BasePath_Get(string tempDir)
+        {
+            var collection = new TempFileCollection(tempDir);
+            if (Directory.Exists(tempDir))
+            {
+                Assert.StartsWith(tempDir, collection.BasePath);
+            }
+            else
+            {
+                Assert.Throws<DirectoryNotFoundException>(() => collection.BasePath);
+                Assert.StartsWith(tempDir, collection.BasePath);
+            }
+        }
+
+        [Fact]
+        public void AddFileExtension()
+        {
+            string tempDirectory = TempDirectory();
+            using (var collection = new TempFileCollection(tempDirectory))
+            {
+                string file = collection.AddExtension("txt");
+                Assert.False(File.Exists(file));
+                Assert.Equal(collection.BasePath + "." + "txt", file);
+            }
+        }
+
+        [Theory]
+        [InlineData(null)]
+        [InlineData("")]
+        public void AddExtension_InvalidFileExtension_ThrowsArgumentException(string fileExtension)
+        {
+            using (var collection = new TempFileCollection())
+            {
+                Assert.Throws<ArgumentException>("fileExtension", () => collection.AddExtension(fileExtension));
+                Assert.Throws<ArgumentException>("fileExtension", () => collection.AddExtension(fileExtension, keepFile: false));
+            }
+        }
+
+        [Theory]
+        [InlineData(true, true)]
+        [InlineData(true, false)]
+        [InlineData(false, true)]
+        [InlineData(false, false)]
+        public void AddFile(bool fileExists, bool keepFile)
+        {
+            string directory = TempDirectory();
+            const string FileName = "file.extension";
+            string filePath = Path.Combine(directory, FileName);
+            if (fileExists)
+            {
+                File.Create(filePath).Dispose();
+            }
+            try
+            {
+                using (var collection = new TempFileCollection(directory))
+                {
+                    // AddFile(fileName) is a misnomer, and should really be AddFile(filePath),
+                    // as only files added with their full path are deleted.
+                    collection.AddFile(filePath, keepFile);
+                    Assert.Equal(fileExists, File.Exists(filePath));
+                }
+
+                Assert.Equal(fileExists && keepFile, File.Exists(filePath));
+            }
+            finally
+            {
+                if (File.Exists(filePath))
+                {
+                    File.Delete(filePath);
+                }
+            }
+        }
+
+        [Theory]
+        [InlineData(true)]
+        [InlineData(false)]
+        public void AddFile_MultipleFiles_DeletesAllIfKeepFilesFalse(bool keepFiles)
+        {
+            string directory = TempDirectory();
+            string filePath1 = Path.Combine(directory, "file1.extension");
+            string filePath2 = Path.Combine(directory, "file2.extension");
+
+            File.Create(filePath1).Dispose();
+            File.Create(filePath2).Dispose();
+
+            try
+            {
+                using (var collection = new TempFileCollection(directory))
+                {
+                    collection.AddFile(filePath1, keepFiles);
+                    collection.AddFile(filePath2, keepFiles);
+                }
+
+                Assert.Equal(keepFiles, File.Exists(filePath1));
+                Assert.Equal(keepFiles, File.Exists(filePath2));
+            }
+            finally
+            {
+                if (File.Exists(filePath1))
+                {
+                    File.Delete(filePath1);
+                }
+                if (File.Exists(filePath2))
+                {
+                    File.Delete(filePath2);
+                }
+            }
+        }
+
+        [Theory]
+        [InlineData(null)]
+        [InlineData("")]
+        public void AddFile_InvalidFileName_ThrowsArgumentException(string fileName)
+        {
+            using (var collection = new TempFileCollection())
+            {
+                Assert.Throws<ArgumentException>("fileName", () => collection.AddFile(fileName, keepFile: false));
+            }
+        }
+
+        [Fact]
+        public void AddFile_DuplicateFileName_ThrowsArgumentException()
+        {
+            using (var collection = new TempFileCollection())
+            {
+                const string FileName = "FileName";
+                collection.AddFile(FileName, keepFile: false);
+                Assert.Throws<ArgumentException>("fileName", () => collection.AddFile(FileName, keepFile: false));
+
+                // Case insensitive
+                Assert.Throws<ArgumentException>("fileName", () => collection.AddFile(FileName.ToLowerInvariant(), keepFile: false));
+            }
+        }
+
+        [Fact]
+        public void GetEnumerator_Empty_ReturnsFalse()
+        {
+            using (var collection = new TempFileCollection())
+            {
+                Assert.False(collection.GetEnumerator().MoveNext());
+            }
+        }
+
+        [Fact]
+        public void CopyTo()
+        {
+            using (var collection = new TempFileCollection())
+            {
+                const int ArrayIndex = 1;
+                const string FileName = "File";
+                collection.AddFile(FileName, keepFile: false);
+
+                var array = new string[ArrayIndex + collection.Count];
+                collection.CopyTo(array, ArrayIndex);
+
+                Assert.Null(array[0]);
+                Assert.Equal(FileName, array[ArrayIndex]);
+            }
+        }
+
+        private static string s_tempDirectory = null;
+        private static string TempDirectory()
+        {
+            if (s_tempDirectory == null)
+            {
+                string tempDirectory = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName());
+                Directory.CreateDirectory(tempDirectory);
+                s_tempDirectory = tempDirectory;
+            }
+            return s_tempDirectory;
+        }
+    }
+}