Merge pull request #20051 from heejaechang/FixRace

fix race on _storage field.

src/Workspaces/Core/Portable/Utilities/ValuesSources/RecoverableWeakValueSource.cs

@@ -65,7 +65,11 @@ public RecoverableWeakValueSource(RecoverableWeakValueSource<T> savedSource)
 
         public override bool TryGetValue(out T value)
         {
-            return _weakInstance.TryGetTarget(out value);
+            // it has 2 fields that can hold onto the value. if we only check weakInstance, we will
+            // return false for the initial case where weakInstance is set to s_noReference even if
+            // value can be retrieved from _recoverySource. so we check both here.
+            return _weakInstance.TryGetTarget(out value) || 
+                   _recoverySource.TryGetValue(out value);
         }
 
         public override T GetValue(CancellationToken cancellationToken)

src/Workspaces/Core/Portable/Workspace/Host/SyntaxTreeFactory/AbstractSyntaxTreeFactoryService.AbstractRecoverableSyntaxRoot.cs

@@ -82,6 +82,8 @@ internal sealed class RecoverableSyntaxRoot<TRoot> : RecoverableWeakValueSource<
                 IRecoverableSyntaxTree<TRoot> containingTree)
                 : base(originalRoot)
             {
+                Contract.ThrowIfNull(originalRoot._storage);
+
                 _service = originalRoot._service;
                 _storage = originalRoot._storage;
                 _containingTree = containingTree;
@@ -89,14 +91,15 @@ internal sealed class RecoverableSyntaxRoot<TRoot> : RecoverableWeakValueSource<
 
             public RecoverableSyntaxRoot<TRoot> WithSyntaxTree(IRecoverableSyntaxTree<TRoot> containingTree)
             {
+                // at this point, we should either have strongly held root or _storage should not be null
                 if (this.TryGetValue(out var root))
                 {
-                    var result = new RecoverableSyntaxRoot<TRoot>(_service, root, containingTree);
-                    result._storage = _storage;
-                    return result;
+                    // we have strongly held root
+                    return new RecoverableSyntaxRoot<TRoot>(_service, root, containingTree);
                 }
                 else
                 {
+                    // we have _storage here. _storage != null is checked inside
                     return new RecoverableSyntaxRoot<TRoot>(this, containingTree);
                 }
             }

src/Workspaces/CoreTest/SolutionTests/SolutionTests.cs

@@ -1465,6 +1465,34 @@ public async Task TestMassiveFileSize()
             }
         }
 
+        [Fact, Trait(Traits.Feature, Traits.Features.Workspace)]
+        [WorkItem(18697, "https://github.com/dotnet/roslyn/issues/18697")]
+        public void TestWithSyntaxTree()
+        {
+            // get one to get to syntax tree factory
+            var dummyProject = CreateNotKeptAliveSolution().AddProject("dummy", "dummy", LanguageNames.CSharp);
+
+            var factory = dummyProject.LanguageServices.SyntaxTreeFactory;
+
+            // create the origin tree
+            var strongTree = factory.ParseSyntaxTree("dummy", dummyProject.ParseOptions, SourceText.From("// emtpy"), CancellationToken.None);
+
+            // create recoverable tree off the original tree
+            var recoverableTree = factory.CreateRecoverableTree(
+                dummyProject.Id, 
+                strongTree.FilePath, 
+                strongTree.Options, 
+                new ConstantValueSource<TextAndVersion>(TextAndVersion.Create(strongTree.GetText(), VersionStamp.Create(), strongTree.FilePath)), 
+                strongTree.GetText().Encoding, 
+                strongTree.GetRoot());
+
+            // create new tree before it ever getting root node
+            var newTree = recoverableTree.WithFilePath("different/dummy");
+
+            // this shouldn't throw
+            var root = newTree.GetRoot();
+        }
+
         private static void GetMultipleProjects(
             out Project csBrokenProject,
             out Project vbNormalProject,