diff --git a/src/Microsoft.TestPlatform.Common/Utilities/AssemblyResolver.cs b/src/Microsoft.TestPlatform.Common/Utilities/AssemblyResolver.cs index 8a75ba57be..033c0c0327 100644 --- a/src/Microsoft.TestPlatform.Common/Utilities/AssemblyResolver.cs +++ b/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 /// private bool _isDisposed; + private Stack? _currentlyResolvingResources; /// /// Assembly resolver for platform @@ -134,29 +136,62 @@ internal void AddSearchDirectories(IEnumerable 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(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) { diff --git a/test/Microsoft.TestPlatform.AcceptanceTests/RecursiveResourcesLookupTests.cs b/test/Microsoft.TestPlatform.AcceptanceTests/RecursiveResourcesLookupTests.cs new file mode 100644 index 0000000000..3978dfef2a --- /dev/null +++ b/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); + } +} diff --git a/test/TestAssets/RecursiveResourceLookupCrash/RecursiveResourceLookupCrash.csproj b/test/TestAssets/RecursiveResourceLookupCrash/RecursiveResourceLookupCrash.csproj new file mode 100644 index 0000000000..eea331c064 Binary files /dev/null and b/test/TestAssets/RecursiveResourceLookupCrash/RecursiveResourceLookupCrash.csproj differ diff --git a/test/TestAssets/RecursiveResourceLookupCrash/UnitTest1.cs b/test/TestAssets/RecursiveResourceLookupCrash/UnitTest1.cs new file mode 100644 index 0000000000..ec8fef250e --- /dev/null +++ b/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) + { + } + } + } +} diff --git a/test/TestAssets/TestAssets.sln b/test/TestAssets/TestAssets.sln index 8fae3a203e..9d6c2fd82f 100644 --- a/test/TestAssets/TestAssets.sln +++ b/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