Skip to content

Commit

Permalink
Merge pull request #15800 from dotnet/revert-15690-Bug296548
Browse files Browse the repository at this point in the history
Revert "Always queue project removal actions"
  • Loading branch information
heejaechang committed Dec 9, 2016
2 parents 974abc9 + 50ae867 commit 55ab603
Show file tree
Hide file tree
Showing 7 changed files with 57 additions and 64 deletions.
50 changes: 0 additions & 50 deletions src/Test/Utilities/Desktop/ObjectReference.cs
Expand Up @@ -2,8 +2,6 @@

using System;
using System.Runtime.CompilerServices;
using System.Threading;
using System.Threading.Tasks;
using Xunit;

namespace Roslyn.Test.Utilities
Expand Down Expand Up @@ -97,54 +95,6 @@ private void ReleaseAndGarbageCollect()
}
}

#region Async
/// <summary>
/// Asserts that the underlying object has been released.
/// </summary>
[MethodImpl(MethodImplOptions.NoInlining)]
public async Task AssertReleasedAsync()
{
await ReleaseAndGarbageCollectAsync().ConfigureAwait(false);

Assert.False(_weakReference.IsAlive, "Reference should have been released but was not.");
}

/// <summary>
/// Asserts that the underlying object is still being held.
/// </summary>
[MethodImpl(MethodImplOptions.NoInlining)]
public async Task AssertHeldAsync()
{
await ReleaseAndGarbageCollectAsync().ConfigureAwait(false);

// Since we are asserting it's still held, if it is held we can just recover our strong reference again
_strongReference = (T)_weakReference.Target;
Assert.True(_strongReference != null, "Reference should still be held.");
}

[MethodImpl(MethodImplOptions.NoInlining)]
private async Task ReleaseAndGarbageCollectAsync()
{
if (_strongReferenceRetrievedOutsideScopedCall)
{
throw new InvalidOperationException($"The strong reference being held by the {nameof(ObjectReference<T>)} was retrieved via a call to {nameof(GetReference)}. Since the CLR might have cached a temporary somewhere in your stack, assertions can no longer be made about the correctness of lifetime.");
}

_strongReference = null;

// We'll loop 1000 times, or until the weak reference disappears. When we're trying to assert that the
// object is released, once the weak reference goes away, we know we're good. But if we're trying to assert
// that the object is held, our only real option is to know to do it "enough" times; but if it goes away then
// we are definitely done.
for (var i = 0; i < 1000 && _weakReference.IsAlive; i++)
{
GC.Collect();
GC.WaitForPendingFinalizers();
await Task.Yield();
}
}
#endregion

/// <summary>
/// Provides the underlying strong refernce to the given action. This method is marked not be inlined, to ensure that no temporaries are left
/// on the stack that might still root the strong reference. The caller must not "leak" the object out of the given action for any lifetime
Expand Down
Expand Up @@ -28,6 +28,9 @@ public void AddingReferenceToProjectMetadataPromotesToProjectReference()
project2.OnImportAdded(@"c:\project1.dll", "project1");

Assert.Equal(true, project2.GetCurrentProjectReferences().Any(pr => pr.ProjectId == project1.Id));

project2.Disconnect();
project1.Disconnect();
}
}

Expand All @@ -50,6 +53,9 @@ public void AddCyclicProjectMetadataReferences()

Assert.Equal(true, project1.GetCurrentProjectReferences().Any(pr => pr.ProjectId == project2.Id));
Assert.Equal(false, project2.GetCurrentProjectReferences().Any(pr => pr.ProjectId == project1.Id));

project2.Disconnect();
project1.Disconnect();
}
}

Expand All @@ -67,6 +73,9 @@ public void AddCyclicProjectReferences()

Assert.Equal(true, project1.GetCurrentProjectReferences().Any(pr => pr.ProjectId == project2.Id));
Assert.Equal(false, project2.GetCurrentProjectReferences().Any(pr => pr.ProjectId == project1.Id));

project2.Disconnect();
project1.Disconnect();
}
}

Expand All @@ -90,6 +99,11 @@ public void AddCyclicProjectReferencesDeep()
Assert.Equal(true, project2.GetCurrentProjectReferences().Any(pr => pr.ProjectId == project3.Id));
Assert.Equal(true, project3.GetCurrentProjectReferences().Any(pr => pr.ProjectId == project4.Id));
Assert.Equal(false, project4.GetCurrentProjectReferences().Any(pr => pr.ProjectId == project1.Id));

project4.Disconnect();
project3.Disconnect();
project2.Disconnect();
project1.Disconnect();
}
}

Expand All @@ -116,6 +130,9 @@ public void AddingProjectReferenceAndUpdateReferenceBinPath()

// Verify project reference updated after bin path change.
Assert.Equal(true, project2.GetCurrentProjectReferences().Any(pr => pr.ProjectId == project1.Id));

project2.Disconnect();
project1.Disconnect();
}
}
}
Expand Down
Expand Up @@ -2,8 +2,6 @@

using System;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.VisualStudio.LanguageServices.UnitTests.ProjectSystemShim.Framework;
using Roslyn.Test.Utilities;
using Xunit;
Expand All @@ -15,7 +13,7 @@ public class LifetimeTests
[WpfFact]
[Trait(Traits.Feature, Traits.Features.ProjectSystemShims)]
[WorkItem(10358, "https://github.com/dotnet/roslyn/issues/10358")]
public async Task DisconnectingAProjectDoesNotLeak()
public void DisconnectingAProjectDoesNotLeak()
{
using (var environment = new TestEnvironment())
{
Expand All @@ -24,7 +22,7 @@ public async Task DisconnectingAProjectDoesNotLeak()
Assert.Single(environment.Workspace.CurrentSolution.Projects);

project.UseReference(p => p.Disconnect());
await project.AssertReleasedAsync().ConfigureAwait(false);
project.AssertReleased();
}
}
}
Expand Down
Expand Up @@ -136,11 +136,7 @@ public VisualStudioProjectTracker(IServiceProvider serviceProvider, HostWorkspac
private void ScheduleForegroundAffinitizedAction(Action action)
{
AssertIsBackground();
ScheduleForegroundAffinitizedAction_NoBackgroundAssert(action);
}

private void ScheduleForegroundAffinitizedAction_NoBackgroundAssert(Action action)
{
lock (_gate)
{
_taskForForegroundAffinitizedActions = _taskForForegroundAffinitizedActions.SafeContinueWith(_ => action(), ForegroundTaskScheduler);
Expand Down Expand Up @@ -368,8 +364,7 @@ private void StartPushingToWorkspaceAndNotifyOfOpenDocuments_Foreground(IEnumera
/// </summary>
internal void RemoveProject(AbstractProject project)
{
// always schedule on queue so removal happens after other queued actions.
ScheduleForegroundAffinitizedAction_NoBackgroundAssert(() => RemoveProject_Foreground(project));
ExecuteOrScheduleForegroundAffinitizedAction(() => RemoveProject_Foreground(project));
}

/// <summary>
Expand Down
Expand Up @@ -22,6 +22,8 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.ProjectSystemShim

Assert.Contains(New KeyValuePair(Of String, Object)("VBC_VER", PredefinedPreprocessorSymbols.CurrentVersionNumber), options.PreprocessorSymbols)
Assert.Contains(New KeyValuePair(Of String, Object)("TARGET", "exe"), options.PreprocessorSymbols)

project.Disconnect()
End Using
End Sub

Expand All @@ -40,6 +42,8 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.ProjectSystemShim
Dim options = DirectCast(workspaceProject.ParseOptions, VisualBasicParseOptions)

Assert.Equal(DocumentationMode.Diagnose, options.DocumentationMode)

project.Disconnect()
End Using
End Sub

Expand All @@ -58,6 +62,8 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.ProjectSystemShim
Dim options = DirectCast(workspaceProject.ParseOptions, VisualBasicParseOptions)

Assert.Equal(DocumentationMode.Parse, options.DocumentationMode)

project.Disconnect()
End Using
End Sub

Expand Down Expand Up @@ -86,6 +92,8 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.ProjectSystemShim
options = DirectCast(workspaceProject.CompilationOptions, VisualBasicCompilationOptions)

Assert.False(options.SpecificDiagnosticOptions.ContainsKey("BC1234"))

project.Disconnect()
End Using
End Sub
End Class
Expand Down
Expand Up @@ -5,7 +5,6 @@ Imports Roslyn.Test.Utilities
Imports Microsoft.VisualStudio.LanguageServices.UnitTests.ProjectSystemShim.Framework
Imports Microsoft.VisualStudio.LanguageServices.UnitTests.ProjectSystemShim.VisualBasicHelpers
Imports Microsoft.CodeAnalysis
Imports System.Threading.Tasks

Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.ProjectSystemShim
Public Class VisualBasicProjectTests
Expand All @@ -28,16 +27,16 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.ProjectSystemShim

<WpfFact()>
<Trait(Traits.Feature, Traits.Features.ProjectSystemShims)>
Public Async Function DisconnectingAProjectDoesNotLeak() As Task
Public Sub DisconnectingAProjectDoesNotLeak()
Using environment = New TestEnvironment()
Dim project = ObjectReference.CreateFromFactory(Function() CreateVisualBasicProject(environment, "Test"))

Assert.Single(environment.Workspace.CurrentSolution.Projects)

project.UseReference(Sub(p) p.Disconnect())

Await project.AssertReleasedAsync().ConfigureAwait(False)
project.AssertReleased()
End Using
End Function
End Sub
End Class
End Namespace
Expand Up @@ -20,6 +20,8 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.ProjectSystemShim
Assert.True(workspaceProject.HasMetadataReference(MockCompilerHost.FullFrameworkCompilerHost.GetWellKnownDllName("Microsoft.VisualBasic.dll")))
Assert.True(workspaceProject.HasMetadataReference(MockCompilerHost.FullFrameworkCompilerHost.GetWellKnownDllName("mscorlib.dll")))
Assert.True(workspaceProject.HasMetadataReference(MockCompilerHost.FullFrameworkCompilerHost.GetWellKnownDllName("System.dll")))

project.Disconnect()
End Using
End Sub

Expand All @@ -38,6 +40,8 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.ProjectSystemShim
Assert.True(workspaceProject.HasMetadataReference(MockCompilerHost.FullFrameworkCompilerHost.GetWellKnownDllName("Microsoft.VisualBasic.dll")))
Assert.True(workspaceProject.HasMetadataReference(MockCompilerHost.FullFrameworkCompilerHost.GetWellKnownDllName("mscorlib.dll")))
Assert.False(workspaceProject.HasMetadataReference(MockCompilerHost.FullFrameworkCompilerHost.GetWellKnownDllName("System.dll")))

project.Disconnect()
End Using
End Sub

Expand All @@ -56,6 +60,8 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.ProjectSystemShim
Assert.False(workspaceProject.HasMetadataReference(MockCompilerHost.FullFrameworkCompilerHost.GetWellKnownDllName("Microsoft.VisualBasic.dll")))
Assert.True(workspaceProject.HasMetadataReference(MockCompilerHost.FullFrameworkCompilerHost.GetWellKnownDllName("mscorlib.dll")))
Assert.True(workspaceProject.HasMetadataReference(MockCompilerHost.FullFrameworkCompilerHost.GetWellKnownDllName("System.dll")))

project.Disconnect()
End Using
End Sub

Expand Down Expand Up @@ -85,6 +91,8 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.ProjectSystemShim
' It should still reference the VB runtime
workspaceProject = environment.Workspace.CurrentSolution.Projects.Single()
Assert.True(workspaceProject.HasMetadataReference(MockCompilerHost.FullFrameworkCompilerHost.GetWellKnownDllName("Microsoft.VisualBasic.dll")))

project.Disconnect()
End Using
End Sub

Expand All @@ -100,6 +108,8 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.ProjectSystemShim
' We should have no references
Dim workspaceProject = environment.Workspace.CurrentSolution.Projects.Single()
Assert.Empty(workspaceProject.MetadataReferences)

project.Disconnect()
End Using
End Sub

Expand Down Expand Up @@ -127,6 +137,8 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.ProjectSystemShim
' It still should be referencing it since we're implicitly adding it as a part of the options
workspaceProject = environment.Workspace.CurrentSolution.Projects.Single()
Assert.True(workspaceProject.HasMetadataReference(MockCompilerHost.FullFrameworkCompilerHost.GetWellKnownDllName("Microsoft.VisualBasic.dll")))

project.Disconnect()
End Using
End Sub

Expand All @@ -145,6 +157,9 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.ProjectSystemShim
project2.AddMetaDataReference("c:\project1.dll", True)

Assert.Equal(True, project2.GetCurrentProjectReferences().Any(Function(pr) pr.ProjectId = project1.Id))

project2.Disconnect()
project1.Disconnect()
End Using
End Sub

Expand All @@ -166,6 +181,9 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.ProjectSystemShim

Assert.Equal(True, project1.GetCurrentProjectReferences().Any(Function(pr) pr.ProjectId = project2.Id))
Assert.Equal(False, project2.GetCurrentProjectReferences().Any(Function(pr) pr.ProjectId = project1.Id))

project2.Disconnect()
project1.Disconnect()
End Using
End Sub

Expand All @@ -182,6 +200,9 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.ProjectSystemShim

Assert.Equal(True, project1.GetCurrentProjectReferences().Any(Function(pr) pr.ProjectId = project2.Id))
Assert.Equal(False, project2.GetCurrentProjectReferences().Any(Function(pr) pr.ProjectId = project1.Id))

project2.Disconnect()
project1.Disconnect()
End Using
End Sub

Expand All @@ -204,6 +225,11 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.ProjectSystemShim
Assert.Equal(True, project2.GetCurrentProjectReferences().Any(Function(pr) pr.ProjectId = project3.Id))
Assert.Equal(True, project3.GetCurrentProjectReferences().Any(Function(pr) pr.ProjectId = project4.Id))
Assert.Equal(False, project4.GetCurrentProjectReferences().Any(Function(pr) pr.ProjectId = project1.Id))

project4.Disconnect()
project3.Disconnect()
project2.Disconnect()
project1.Disconnect()
End Using
End Sub

Expand Down

0 comments on commit 55ab603

Please sign in to comment.