Fix recursive resource lookup

src/Microsoft.TestPlatform.Common/Utilities/AssemblyResolver.cs

@@ -2,6 +2,7 @@
 // Licensed under the MIT license. See LICENSE file in the project root for full license information.
 
 using System;
+using System.Collections;
 using System.Collections.Generic;
 using System.Diagnostics;
 using System.IO;
@@ -31,6 +32,7 @@ internal class AssemblyResolver : IDisposable
     /// Specifies whether the resolver is disposed or not
     /// </summary>
     private bool _isDisposed;
+    private Stack<string>? _currentlyResolvingResources;
 
     /// <summary>
     /// Assembly resolver for platform
@@ -134,29 +136,62 @@ internal void AddSearchDirectories(IEnumerable<string> directories)
                     var assemblyPath = Path.Combine(dir, requestedName.Name + extension);
                     try
                     {
-                        if (!File.Exists(assemblyPath))
+                        var isResource = requestedName.Name.EndsWith(".resources");
+                        bool pushed = false;
+                        try
                         {
-                            EqtTrace.Info("AssemblyResolver.OnResolve: {0}: Assembly path does not exist: '{1}', returning.", args.Name, assemblyPath);
+                            if (isResource)
+                            {
+                                // Check for recursive resource lookup.
+                                // This can happen when we are on non-english locale, and we try to load mscorlib.resources
+                                // (or potentially some other resources). This will trigger a new Resolve and call the method
+                                // we are currently in. If then some code in this Resolve method (like File.Exists) will again
+                                // try to access mscorlib.resources it will end up recursing forever.
 
-                            continue;
-                        }
+                                if (_currentlyResolvingResources != null && _currentlyResolvingResources.Count > 0 && _currentlyResolvingResources.Contains(assemblyPath))
+                                {
+                                    EqtTrace.Info("AssemblyResolver.OnResolve: {0}: Assembly is searching for itself recursively: '{1}', returning as not found.", args.Name, assemblyPath);
+                                    _resolvedAssemblies[args.Name] = null;
+                                    return null;
+                                }
 
-                        AssemblyName foundName = _platformAssemblyLoadContext.GetAssemblyNameFromPath(assemblyPath);
+                                _currentlyResolvingResources ??= new Stack<string>(4);
+                                _currentlyResolvingResources.Push(assemblyPath);
+                                pushed = true;
+                            }
 
-                        if (!RequestedAssemblyNameMatchesFound(requestedName, foundName))
-                        {
-                            EqtTrace.Info("AssemblyResolver.OnResolve: {0}: File exists but version/public key is wrong. Try next extension.", args.Name);
-                            continue;   // File exists but version/public key is wrong. Try next extension.
-                        }
+                            if (!File.Exists(assemblyPath))
+                            {
+                                EqtTrace.Info("AssemblyResolver.OnResolve: {0}: Assembly path does not exist: '{1}', returning.", args.Name, assemblyPath);
+
+                                continue;
+                            }
+
+                            AssemblyName foundName = _platformAssemblyLoadContext.GetAssemblyNameFromPath(assemblyPath);
 
-                        EqtTrace.Info("AssemblyResolver.OnResolve: {0}: Loading assembly '{1}'.", args.Name, assemblyPath);
+                            if (!RequestedAssemblyNameMatchesFound(requestedName, foundName))
+                            {
+                                EqtTrace.Info("AssemblyResolver.OnResolve: {0}: File exists but version/public key is wrong. Try next extension.", args.Name);
+                                continue;   // File exists but version/public key is wrong. Try next extension.
+                            }
 
-                        assembly = _platformAssemblyLoadContext.LoadAssemblyFromPath(assemblyPath);
-                        _resolvedAssemblies[args.Name] = assembly;
+                            EqtTrace.Info("AssemblyResolver.OnResolve: {0}: Loading assembly '{1}'.", args.Name, assemblyPath);
 
-                        EqtTrace.Info("AssemblyResolver.OnResolve: Resolved assembly: {0}, from path: {1}", args.Name, assemblyPath);
+                            assembly = _platformAssemblyLoadContext.LoadAssemblyFromPath(assemblyPath);
+                            _resolvedAssemblies[args.Name] = assembly;
 
-                        return assembly;
+                            EqtTrace.Info("AssemblyResolver.OnResolve: Resolved assembly: {0}, from path: {1}", args.Name, assemblyPath);
+
+                            return assembly;
+                        }
+                        finally
+                        {
+                            if (isResource && pushed)
+                            {
+                                _currentlyResolvingResources?.Pop();
+                            }
+
+                        }
                     }
                     catch (FileLoadException ex)
                     {

test/Microsoft.TestPlatform.AcceptanceTests/RecursiveResourcesLookupTests.cs

@@ -0,0 +1,27 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT license. See LICENSE file in the project root for full license information.
+
+using Microsoft.TestPlatform.TestUtilities;
+using Microsoft.VisualStudio.TestTools.UnitTesting;
+
+namespace Microsoft.TestPlatform.AcceptanceTests;
+
+[TestClass]
+public class RecursiveResourcesLookupTests : AcceptanceTestBase
+{
+    [TestMethod]
+    // This only fails on .NET Framework, and it fails in teshtost, so no need to double check with
+    // two different runners.
+    [NetFullTargetFrameworkDataSource(useCoreRunner: false)]
+    public void RunsToCompletionWhenJapaneseResourcesAreLookedUpForMSCorLib(RunnerInfo runnerInfo)
+    {
+        SetTestEnvironment(_testEnvironment, runnerInfo);
+
+        var assemblyPath = BuildMultipleAssemblyPath("RecursiveResourceLookupCrash.dll");
+        var arguments = PrepareArguments(assemblyPath, null, null, FrameworkArgValue, runnerInfo.InIsolationValue, resultsDirectory: TempDirectory.Path);
+        InvokeVsTest(arguments);
+
+        // If we don't short-circuit the recursion correctly testhost will crash with StackOverflow.
+        ValidateSummaryStatus(passed: 1, failed: 0, 0);
+    }
+}

test/TestAssets/RecursiveResourceLookupCrash/UnitTest1.cs

@@ -0,0 +1,39 @@
+using System;
+using System.Globalization;
+using System.IO;
+using System.IO.IsolatedStorage;
+using System.Threading;
+
+using Microsoft.VisualStudio.TestTools.UnitTesting;
+
+namespace RecursiveResourceLookupCrash
+{
+    [TestClass]
+    public class RecursiveResourceLookupCrashTests
+    {
+        [TestInitialize]
+        public void TestInitialize()
+        {
+            // You need to set non-English culture explicitly to reproduce recursive resource
+            // lookup bug in English environment.
+            Thread.CurrentThread.CurrentUICulture = new CultureInfo("ja-JP");
+        }
+
+        [TestMethod]
+        public void CrashesOnResourcesLookupWhenNotHandledByAssemblyResolver()
+        {
+            try
+            {
+                // This will internally trigger file not found exception, and try to find ja-JP resources
+                // for the string, which will trigger Resolve in AssemblyResolver, which will
+                // use File.Exists call and that will trigger another round of looking up ja-JP
+                // resources, until this is detected by .NET Framework, and Environment.FailFast
+                // is called to crash the testhost.
+                var stream = new IsolatedStorageFileStream("non-existent-filename", FileMode.Open);
+            }
+            catch (Exception)
+            {
+            }
+        }
+    }
+}

test/TestAssets/TestAssets.sln

@@ -124,6 +124,8 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Tools", "Tools\Tools.csproj
 EndProject
 Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Perfy.TestAdapter", "performance\Perfy.TestAdapter\Perfy.TestAdapter.csproj", "{71BF7EC9-7BEE-4038-8F4E-87032FA4E995}"
 EndProject
+Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "RecursiveResourceLookupCrash", "RecursiveResourceLookupCrash\RecursiveResourceLookupCrash.csproj", "{9B5BDF7D-5816-47C6-8CFD-E45B152CA35F}"
+EndProject
 Global
 	GlobalSection(SolutionConfigurationPlatforms) = preSolution
 		Debug|Any CPU = Debug|Any CPU
@@ -806,6 +808,18 @@ Global
 		{71BF7EC9-7BEE-4038-8F4E-87032FA4E995}.Release|x64.Build.0 = Release|Any CPU
 		{71BF7EC9-7BEE-4038-8F4E-87032FA4E995}.Release|x86.ActiveCfg = Release|Any CPU
 		{71BF7EC9-7BEE-4038-8F4E-87032FA4E995}.Release|x86.Build.0 = Release|Any CPU
+		{9B5BDF7D-5816-47C6-8CFD-E45B152CA35F}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
+		{9B5BDF7D-5816-47C6-8CFD-E45B152CA35F}.Debug|Any CPU.Build.0 = Debug|Any CPU
+		{9B5BDF7D-5816-47C6-8CFD-E45B152CA35F}.Debug|x64.ActiveCfg = Debug|Any CPU
+		{9B5BDF7D-5816-47C6-8CFD-E45B152CA35F}.Debug|x64.Build.0 = Debug|Any CPU
+		{9B5BDF7D-5816-47C6-8CFD-E45B152CA35F}.Debug|x86.ActiveCfg = Debug|Any CPU
+		{9B5BDF7D-5816-47C6-8CFD-E45B152CA35F}.Debug|x86.Build.0 = Debug|Any CPU
+		{9B5BDF7D-5816-47C6-8CFD-E45B152CA35F}.Release|Any CPU.ActiveCfg = Release|Any CPU
+		{9B5BDF7D-5816-47C6-8CFD-E45B152CA35F}.Release|Any CPU.Build.0 = Release|Any CPU
+		{9B5BDF7D-5816-47C6-8CFD-E45B152CA35F}.Release|x64.ActiveCfg = Release|Any CPU
+		{9B5BDF7D-5816-47C6-8CFD-E45B152CA35F}.Release|x64.Build.0 = Release|Any CPU
+		{9B5BDF7D-5816-47C6-8CFD-E45B152CA35F}.Release|x86.ActiveCfg = Release|Any CPU
+		{9B5BDF7D-5816-47C6-8CFD-E45B152CA35F}.Release|x86.Build.0 = Release|Any CPU
 	EndGlobalSection
 	GlobalSection(SolutionProperties) = preSolution
 		HideSolutionNode = FALSE