Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove usage of weak-value-source for smart-open-scope references #58212

Merged
merged 2 commits into from
Dec 10, 2021
Merged

Remove usage of weak-value-source for smart-open-scope references #58212

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9f03216
Remove usage of weak-value-source for smart-open-scope references
CyrusNajmabadi Dec 9, 2021
5fb30c7
Make required
CyrusNajmabadi Dec 10, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 2 additions & 5 deletions ...entation/ProjectSystem/MetadataReferences/VisualStudioMetadataReferenceManager.Factory.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,9 @@
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

#nullable disable

using System;
using System.Composition;
using System.Threading;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Host;
using Microsoft.CodeAnalysis.Host.Mef;
using Microsoft.VisualStudio.Shell;
Expand All @@ -19,7 +16,7 @@ namespace Microsoft.VisualStudio.LanguageServices.Implementation.ProjectSystem
[ExportWorkspaceServiceFactory(typeof(VisualStudioMetadataReferenceManager), ServiceLayer.Host), Shared]
internal class VisualStudioMetadataReferenceManagerFactory : IWorkspaceServiceFactory
{
private VisualStudioMetadataReferenceManager _singleton;
private VisualStudioMetadataReferenceManager? _singleton;
private readonly IServiceProvider _serviceProvider;

[ImportingConstructor]
Expand All @@ -32,7 +29,7 @@ public IWorkspaceService CreateService(HostWorkspaceServices workspaceServices)
if (_singleton == null)
{
var temporaryStorage = workspaceServices.GetService<ITemporaryStorageService>();
Interlocked.CompareExchange(ref _singleton, new VisualStudioMetadataReferenceManager(workspaceServices.Workspace, _serviceProvider, temporaryStorage), null);
Interlocked.CompareExchange(ref _singleton, new VisualStudioMetadataReferenceManager(_serviceProvider, temporaryStorage), null);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❗ This incorrectly provides a VisualStudioMetadataReferenceManager using the ITemporaryStorageService from an unrelated workspace. The cache field needs to be removed.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This bug isn't introduced by this pull request, but if you aren't fixing it here please file a bug for the 17.1 release.

}

return _singleton;
Expand Down
8 changes: 1 addition & 7 deletions ...f/Implementation/ProjectSystem/MetadataReferences/VisualStudioMetadataReferenceManager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ internal sealed partial class VisualStudioMetadataReferenceManager : IWorkspaceS

private readonly MetadataCache _metadataCache;
private readonly ImmutableArray<string> _runtimeDirectories;
private readonly Workspace _workspace;
private readonly ITemporaryStorageService _temporaryStorageService;

internal IVsXMLMemberIndexService XmlMemberIndexService { get; }
Expand All @@ -53,7 +52,6 @@ internal sealed partial class VisualStudioMetadataReferenceManager : IWorkspaceS
private readonly ReaderWriterLockSlim _readerWriterLock = new();

internal VisualStudioMetadataReferenceManager(
Workspace workspace,
IServiceProvider serviceProvider,
ITemporaryStorageService temporaryStorageService)
{
Expand All @@ -68,7 +66,6 @@ internal VisualStudioMetadataReferenceManager(

FileChangeService = (IVsFileChangeEx)serviceProvider.GetService(typeof(SVsFileChangeEx));
Assumes.Present(FileChangeService);
_workspace = workspace;
_temporaryStorageService = temporaryStorageService;
Assumes.Present(_temporaryStorageService);
}
Expand Down Expand Up @@ -130,10 +127,7 @@ internal Metadata GetMetadata(string fullPath, DateTime snapshotTimestamp)

if (VsSmartScopeCandidate(key.FullPath) && TryCreateAssemblyMetadataFromMetadataImporter(key, out var newMetadata))
{
ValueSource<Optional<AssemblyMetadata>> metadataValueSource = _workspace.Options.GetOption(WorkspaceConfigurationOptions.DisableReferenceManagerWeakRuntimeReferences)
? new ConstantValueSource<Optional<AssemblyMetadata>>(newMetadata)
: new WeakValueSource<AssemblyMetadata>(newMetadata);

var metadataValueSource = new ConstantValueSource<Optional<AssemblyMetadata>>(newMetadata);
if (!_metadataCache.GetOrAddMetadata(key, metadataValueSource, out metadata))
{
newMetadata.Dispose();
Expand Down
8 changes: 0 additions & 8 deletions src/Workspaces/Core/Portable/Workspace/WorkspaceConfigurationOptions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,6 @@ internal class WorkspaceConfigurationOptions : IOptionProvider
nameof(WorkspaceConfigurationOptions), nameof(DisableProjectCacheService), defaultValue: false,
new FeatureFlagStorageLocation("Roslyn.DisableProjectCacheService"));

/// <summary>
/// Disables holding onto the assembly references for runtime (not user/nuget/etc.) dlls weakly.
/// </summary>
public static readonly Option<bool> DisableReferenceManagerWeakRuntimeReferences = new(
nameof(WorkspaceConfigurationOptions), nameof(DisableReferenceManagerWeakRuntimeReferences), defaultValue: false,
new FeatureFlagStorageLocation("Roslyn.DisableReferenceManagerWeakRuntimeReferences"));

/// <summary>
/// Disables holding onto the assembly references for runtime (not user/nuget/etc.) dlls weakly.
/// </summary>
Expand All @@ -43,7 +36,6 @@ internal class WorkspaceConfigurationOptions : IOptionProvider
ImmutableArray<IOption> IOptionProvider.Options { get; } = ImmutableArray.Create<IOption>(
DisableRecoverableTrees,
DisableProjectCacheService,
DisableReferenceManagerWeakRuntimeReferences,
DisableCompilationTrackerWeakCompilationReferences);

[ImportingConstructor]
Expand Down