Skip to content

Commit

Permalink
ResultProvider: Consume DkmClrModuleInstance.GetMetaDataImportHolder (#…
Browse files Browse the repository at this point in the history
…76395)

Previous to this change, the ResultProvider would use `DkmClrModuleInstance.GetMetaDataImport`, but did NOT use Marshal.ReleaseComObject on the result. This could result in locked files if the RCW took a long time to be finalized.

This PR switches to use a new `GetMetaDataImportHolder` API that adds an IDisposable wrapper struct to take care of this cleanup.

This PR requires VS 17.13 Build 35612.30 or newer.
  • Loading branch information
gregg-miskelly authored Jan 8, 2025
1 parent a3723c4 commit 095410a
Show file tree
Hide file tree
Showing 4 changed files with 44 additions and 6 deletions.
4 changes: 2 additions & 2 deletions eng/Directory.Packages.props
Original file line number Diff line number Diff line change
Expand Up @@ -122,8 +122,8 @@
VS Debugger
-->
<PackageVersion Include="Microsoft.VisualStudio.Debugger.Contracts" Version="17.13.0-beta.24561.1" />
<PackageVersion Include="Microsoft.VisualStudio.Debugger.Engine-implementation" Version="17.13.1110101-preview" />
<PackageVersion Include="Microsoft.VisualStudio.Debugger.Metadata-implementation" Version="17.13.1110101-preview" />
<PackageVersion Include="Microsoft.VisualStudio.Debugger.Engine-implementation" Version="17.13.1121201-preview" />
<PackageVersion Include="Microsoft.VisualStudio.Debugger.Metadata-implementation" Version="17.13.1121201-preview" />

<!--
VS .NET Runtime
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -182,18 +182,18 @@ string IDkmClrFullNameProvider2.GetClrNameForLocalVariable(DkmInspectionContext

string IDkmClrFullNameProvider2.GetClrNameForField(DkmInspectionContext inspectionContext, DkmClrModuleInstance moduleInstance, int fieldToken)
{
var import = (IMetadataImport)moduleInstance.GetMetaDataImport();
using var importHolder = moduleInstance.GetMetaDataImportHolder();

// Just get some of information about properties. Get rest later only if needed.
int hr = import.GetFieldProps(fieldToken, out _, null, 0, out var nameLength, out _, out _, out _, out _, out _, out _);
int hr = importHolder.Value.GetFieldProps(fieldToken, out _, null, 0, out var nameLength, out _, out _, out _, out _, out _, out _);
const int S_OK = 0;
if (hr != S_OK)
{
throw new ArgumentException("Invalid field token.", nameof(fieldToken));
}

var sb = new StringBuilder(nameLength);
hr = import.GetFieldProps(fieldToken, out _, sb, sb.Capacity, out _, out _, out _, out _, out _, out _, out _);
hr = importHolder.Value.GetFieldProps(fieldToken, out _, sb, sb.Capacity, out _, out _, out _, out _, out _, out _, out _);
if (hr != S_OK)
{
throw new DkmException((DkmExceptionCode)hr);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ internal int ResolveTypeNameFailures
get { return _resolveTypeNameFailures; }
}

public object GetMetaDataImport() => new MetadataImportMock(Assembly);
public DkmMetadataImportHolder GetMetaDataImportHolder() => new DkmMetadataImportHolder(new MetadataImportMock(Assembly));

private class MetadataImportMock : IMetadataImport
{
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System;
using Microsoft.MetadataReader;

#nullable enable

namespace Microsoft.VisualStudio.Debugger.Clr
{
/// <summary>
/// Wrapper around a <see cref="Microsoft.MetadataReader.IMetadataImport"/> interface
/// reference that provides a easy way to release the reference to the underlying
/// COM object. Instances of this struct should always be disposed.
/// </summary>
public readonly struct DkmMetadataImportHolder : IDisposable
{
/// <summary>
/// The underlying IMetaDataImport interface reference
/// </summary>
public IMetadataImport Value { get; }

/// <summary>
/// Creates a new instance of <see cref="DkmMetadataImportHolder"/>.
/// </summary>
/// <param name="value">[In] Implementation of IMetadataImport to wrap</param>
public DkmMetadataImportHolder(IMetadataImport value)
{
this.Value = value ?? throw new ArgumentNullException(nameof(value));
}

void IDisposable.Dispose()
{
// Mock implementation does nothing
}
}
}

0 comments on commit 095410a

Please sign in to comment.