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

Revert "Fix leak of assembly metadata instances" #73991

Closed
wants to merge 1 commit into from
Closed

Revert "Fix leak of assembly metadata instances" #73991

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
39 changes: 4 additions & 35 deletions ...lStudio/Core/Def/ProjectSystem/MetadataReferences/VisualStudioMetadataReferenceManager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,7 @@
using System.Runtime.InteropServices;
using System.Threading;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Collections.Internal;
using Microsoft.CodeAnalysis.Host;
using Microsoft.CodeAnalysis.Shared.Collections;
using Microsoft.VisualStudio.Shell.Interop;
using Roslyn.Utilities;

Expand Down Expand Up @@ -50,7 +48,7 @@ internal sealed partial class VisualStudioMetadataReferenceManager : IWorkspaceS
/// <summary>
/// Access locked with <see cref="_metadataCacheLock"/>.
/// </summary>
private readonly Dictionary<FileKey, WeakReference<AssemblyMetadata>> _metadataCache = [];
private readonly Dictionary<FileKey, AssemblyMetadata> _metadataCache = [];

private readonly ImmutableArray<string> _runtimeDirectories;
private readonly TemporaryStorageService _temporaryStorageService;
Expand Down Expand Up @@ -92,12 +90,8 @@ public void Dispose()

private bool TryGetMetadata(FileKey key, [NotNullWhen(true)] out AssemblyMetadata? metadata)
{
metadata = null;
lock (_metadataCacheLock)
{
return _metadataCache.TryGetValue(key, out var weakMetadata) &&
weakMetadata.TryGetTarget(out metadata);
}
return _metadataCache.TryGetValue(key, out metadata);
}

public IReadOnlyList<TemporaryStorageStreamHandle>? GetStorageHandles(string fullPath, DateTime snapshotTimestamp)
Expand Down Expand Up @@ -157,16 +151,14 @@ internal Metadata GetMetadata(string fullPath, DateTime snapshotTimestamp)
// Now try to create and add the metadata to the cache. If we fail to add it (because some other thread
// beat us to this), then Dispose the metadata we just created and will return the existing metadata
// instead.
if (_metadataCache.TryGetValue(key, out var weakCachedMetadata) &&
weakCachedMetadata.TryGetTarget(out var cachedMetadata))
if (_metadataCache.TryGetValue(key, out var cachedMetadata))
{
metadata.Dispose();
return cachedMetadata;
}

// don't use "Add" since key might already exist with already released metadata
_metadataCache[key] = new WeakReference<AssemblyMetadata>(metadata);
ClearReleasedMetadata_NoLock();
_metadataCache[key] = metadata;
return metadata;
}
}
Expand All @@ -184,29 +176,6 @@ AssemblyMetadata GetMetadataWorker(string fullPath)

return metadata;
}

void ClearReleasedMetadata_NoLock()
{
Contract.ThrowIfFalse(Monitor.IsEntered(_metadataCacheLock));

var count = _metadataCache.Count;

// Cleanup when we hit powers of two. This way we don't have to walk too often. But only when we've really
// grown. the dictionary by a substantial amount since the last time we cleaned up.
if (count == (1 << SegmentedArraySortUtils.Log2((uint)count)))
{
using var keysToRemove = TemporaryArray<FileKey>.Empty;

foreach (var (fileKey, weakMetadata) in _metadataCache)
{
if (!weakMetadata.TryGetTarget(out _))
keysToRemove.Add(fileKey);
}

foreach (var key in keysToRemove)
_metadataCache.Remove(key);
}
}
}

private static (ModuleMetadata metadata, TemporaryStorageStreamHandle storageHandle) GetMetadataFromTemporaryStorage(
Expand Down
Loading