Skip to content

Fix ISB001 misfires on proxy fields that are disposed#428

Open
Copilot wants to merge 4 commits intomainfrom
copilot/fix-isb001-misfire-issue
Open

Fix ISB001 misfires on proxy fields that are disposed#428
Copilot wants to merge 4 commits intomainfrom
copilot/fix-isb001-misfire-issue

Conversation

Copy link
Copy Markdown

Copilot AI commented Feb 24, 2026

ISB001 false-positives when proxy fields are properly disposed in override Dispose(bool) methods or explicit IDisposable.Dispose() implementations.

The root cause is that three ImmutableArray<T>.Contains() calls used default reference equality instead of SymbolEqualityComparer to compare Roslyn symbols, which can fail to match symbols in complex compilation environments (e.g., VS extensions with many assembly references):

  1. AllInterfaces.Contains(idisposable) — controls whether Dispose methods are analyzed for the type. If this comparison fails, AnalyzeDisposeMethods is never registered, so no Dispose method (including override Dispose(bool)) is analyzed.
  2. getProxyAsyncMethods.Contains(...) — identifies GetProxyAsync calls.
  3. ExplicitInterfaceImplementations.Contains(disposeMethod) — recognizes explicit IDisposable.Dispose() implementations.

Changes

  • AllInterfaces.Contains(idisposable)AllInterfaces.Any(i => SymbolEqualityComparer.Default.Equals(i, idisposable)) — ensures Dispose method analysis is registered even when IDisposable is inherited from a base type
  • getProxyAsyncMethods.Contains(...) → uses SymbolEqualityComparer.Default.Equals() — consistent symbol comparison for GetProxyAsync identification
  • ExplicitInterfaceImplementations.Contains(disposeMethod) → uses SymbolEqualityComparer.Default.Equals() — ensures explicit IDisposable.Dispose() implementations are recognized
  • 2 new testsoverride Dispose(bool) with base class providing IDisposable, and explicit IDisposable.Dispose() with cast-based disposal pattern
Original prompt

This section details on the original issue you should resolve

<issue_title>ISB001 misfires on proxy fields that are disposed</issue_title>
<issue_description>```cs
private ITerminalService terminalService;

    protected override void Dispose(bool disposing)
    {
        base.Dispose(disposing);
        foreach (var token in profferTokens)
            token?.Dispose();

        this.serviceBrokerClient?.Dispose();
        (this.terminalService as IDisposable)?.Dispose();
    }

The field is disposed, but the analyzer still emits this:
error ISB001: The proxy stored in "LinuxConnectionManagerPackage.terminalService" must be disposed by an IDisposable.Dispose or Dispose(bool) method on the "LinuxConnectionManagerPackage" type.</issue_description>

## Comments on the Issue (you are @copilot in this section)

<comments>
<comment_new><author>@AArnott</author><body>
Luca also reported:

I hit this as well, had to explicitly suppress this warning in code, e.g.:

```cs
    internal class RenameRefactorCommand : IVsPreviewChangesEngine, IDisposable
    {
        line110: private readonly Microsoft.VisualStudio.Threading.AsyncLazy<ServiceBrokerClient> serviceBrokerClientLazy;

        void IDisposable.Dispose()
        {
            this.serviceBrokerClientLazy.DisposeValue();
        }
    }

C:\vs\src\vc\designtime\Managed\Rename.cs(110,90): error ISB001: The proxy stored in "RenameRefactorCommand.serviceBrokerClientLazy" must be disposed by an IDisposable.Dispose or Dispose(bool) method on the "RenameRefactorCommand" type. (https://github.com/microsoft/vs-servicehub/blob/3423437139210283925dbc2462d5637c9abad978/doc/analyzers/ISB001.md) [C:\vs\src\vc\designtime\Managed\dtmanaged_21vxelbh_wpftmp.csproj]</comment_new>


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI and others added 2 commits February 24, 2026 16:56
…gned to a field

When GetProxyAsync or new ServiceBrokerClient is inside a lambda that is
assigned to a field (e.g., AsyncLazy<ServiceBrokerClient>), FindAncestors
incorrectly crossed the anonymous function boundary and attributed the proxy
to the outer field. This caused ISB001 to require disposal of a field that
doesn't directly hold a proxy.

The fix prevents EnsureAssignedValueIsDisposed from crossing anonymous
function (lambda) or local function boundaries when looking for ancestor
assignments.

Co-authored-by: AArnott <3548+AArnott@users.noreply.github.com>
Co-authored-by: AArnott <3548+AArnott@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix ISB001 misfire on disposed proxy fields Fix ISB001 misfires when proxy is created inside a lambda assigned to a field Feb 24, 2026
@AArnott AArnott marked this pull request as ready for review February 24, 2026 18:30
@AArnott
Copy link
Copy Markdown
Member

AArnott commented Feb 24, 2026

@copilot Where did this lambdas handling change come from? The issue you were supposed to fix mentioned nothing about lambdas. It was about Dispose methods that were override or explicit interface implementations.

Please start from scratch and focus on the problem identified by the issue.

…isons

Replace ImmutableArray<T>.Contains() calls (which use default reference
equality) with SymbolEqualityComparer.Default.Equals() for three critical
symbol comparisons:

1. AllInterfaces.Contains(idisposable) - controls whether Dispose methods
   are analyzed for the type (affects override Dispose(bool) recognition)
2. getProxyAsyncMethods.Contains(...) - identifies GetProxyAsync calls
3. ExplicitInterfaceImplementations.Contains(disposeMethod) - recognizes
   explicit IDisposable.Dispose() implementations

Also adds two new tests for override Dispose(bool) with base class and
explicit interface IDisposable.Dispose() with cast-based disposal.

Co-authored-by: AArnott <3548+AArnott@users.noreply.github.com>
Copy link
Copy Markdown
Author

Copilot AI commented Feb 24, 2026

@copilot Where did this lambdas handling change come from? The issue you were supposed to fix mentioned nothing about lambdas. It was about Dispose methods that were override or expl...

Reverted the lambda changes and started from scratch. The root cause is that three ImmutableArray<T>.Contains() calls use default reference equality instead of SymbolEqualityComparer to compare Roslyn symbols:

  1. AllInterfaces.Contains(idisposable) — if this fails, AnalyzeDisposeMethods is never registered, so no Dispose method (including override Dispose(bool)) is analyzed
  2. ExplicitInterfaceImplementations.Contains(disposeMethod) — if this fails, explicit IDisposable.Dispose() methods aren't recognized
  3. getProxyAsyncMethods.Contains(...) — same issue for GetProxyAsync identification

Fixed all three to use SymbolEqualityComparer.Default.Equals(), consistent with how the rest of the analyzer compares symbols. Added tests for both override Dispose(bool) and explicit IDisposable.Dispose() patterns. See commit 69d8bf4.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • 5zivsblobprodwus217.vsblob.vsassets.io
    • Triggering command: /opt/hostedtoolcache/dotnet/dotnet dotnet restore --no-dependencies /home/REDACTED/work/vs-servicehub/.codeql-scratch/dbs/csharp/working/F7D2419353110B6F32053972B32A361D/missingpackages_workingdir --packages /home/REDACTED/work/vs-servicehub/.codeql-scratch/dbs/csharp/working/missingpackages /p:DisableImplicitNuGetFallbackFolder=true --verbosity normal --configfile /home/REDACTED/work/vs-servicehub/.codeql-scratch/dbs/csharp/working/nugetconfig/nuget.config --force (dns block)
  • cffvsblobprodwus218.vsblob.vsassets.io
    • Triggering command: /opt/hostedtoolcache/dotnet/dotnet dotnet restore --no-dependencies /home/REDACTED/work/vs-servicehub/.codeql-scratch/dbs/csharp/working/F7D2419353110B6F32053972B32A361D/missingpackages_workingdir --packages /home/REDACTED/work/vs-servicehub/.codeql-scratch/dbs/csharp/working/missingpackages /p:DisableImplicitNuGetFallbackFolder=true --verbosity normal --configfile /home/REDACTED/work/vs-servicehub/.codeql-scratch/dbs/csharp/working/nugetconfig/nuget.config --force (dns block)
  • etjvsblobprodwus2172.vsblob.vsassets.io
    • Triggering command: /opt/hostedtoolcache/dotnet/dotnet dotnet restore --no-dependencies /home/REDACTED/work/vs-servicehub/.codeql-scratch/dbs/csharp/working/F7D2419353110B6F32053972B32A361D/missingpackages_workingdir --packages /home/REDACTED/work/vs-servicehub/.codeql-scratch/dbs/csharp/working/missingpackages /p:DisableImplicitNuGetFallbackFolder=true --verbosity normal --configfile /home/REDACTED/work/vs-servicehub/.codeql-scratch/dbs/csharp/working/nugetconfig/nuget.config --force (dns block)
  • kafvsblobprodwus2165.vsblob.vsassets.io
    • Triggering command: /opt/hostedtoolcache/dotnet/dotnet dotnet restore --no-dependencies /home/REDACTED/work/vs-servicehub/.codeql-scratch/dbs/csharp/working/B42A990C1810BF03B6D8FCD090EC1B53/missingpackages_workingdir --packages /home/REDACTED/work/vs-servicehub/.codeql-scratch/dbs/csharp/working/missingpackages /p:DisableImplicitNuGetFallbackFolder=true --verbosity normal --configfile /home/REDACTED/work/vs-servicehub/.codeql-scratch/dbs/csharp/working/nugetconfig/nuget.config --force (dns block)
    • Triggering command: /opt/hostedtoolcache/dotnet/dotnet dotnet restore --no-dependencies /home/REDACTED/work/vs-servicehub/.codeql-scratch/dbs/csharp/working/919C5216BC25D016B281D430B562FFA1/missingpackages_workingdir --packages /home/REDACTED/work/vs-servicehub/.codeql-scratch/dbs/csharp/working/missingpackages /p:DisableImplicitNuGetFallbackFolder=true --verbosity normal --configfile /home/REDACTED/work/vs-servicehub/.codeql-scratch/dbs/csharp/working/nugetconfig/nuget.config --force (dns block)
  • ldhvsblobprodwus2153.vsblob.vsassets.io
    • Triggering command: /opt/hostedtoolcache/dotnet/dotnet dotnet restore --no-dependencies /home/REDACTED/work/vs-servicehub/.codeql-scratch/dbs/csharp/working/F7D2419353110B6F32053972B32A361D/missingpackages_workingdir --packages /home/REDACTED/work/vs-servicehub/.codeql-scratch/dbs/csharp/working/missingpackages /p:DisableImplicitNuGetFallbackFolder=true --verbosity normal --configfile /home/REDACTED/work/vs-servicehub/.codeql-scratch/dbs/csharp/working/nugetconfig/nuget.config --force (dns block)
  • n6wvsblobprodwus2123.vsblob.vsassets.io
    • Triggering command: /opt/hostedtoolcache/dotnet/dotnet dotnet restore --no-dependencies /home/REDACTED/work/vs-servicehub/.codeql-scratch/dbs/csharp/working/F7D2419353110B6F32053972B32A361D/missingpackages_workingdir --packages /home/REDACTED/work/vs-servicehub/.codeql-scratch/dbs/csharp/working/missingpackages /p:DisableImplicitNuGetFallbackFolder=true --verbosity normal --configfile /home/REDACTED/work/vs-servicehub/.codeql-scratch/dbs/csharp/working/nugetconfig/nuget.config --force (dns block)
  • ndavsblobprodwus219.vsblob.vsassets.io
    • Triggering command: /opt/hostedtoolcache/dotnet/dotnet dotnet restore --no-dependencies /home/REDACTED/work/vs-servicehub/.codeql-scratch/dbs/csharp/working/F7D2419353110B6F32053972B32A361D/missingpackages_workingdir --packages /home/REDACTED/work/vs-servicehub/.codeql-scratch/dbs/csharp/working/missingpackages /p:DisableImplicitNuGetFallbackFolder=true --verbosity normal --configfile /home/REDACTED/work/vs-servicehub/.codeql-scratch/dbs/csharp/working/nugetconfig/nuget.config --force (dns block)
  • ox9vsblobprodwus2149.vsblob.vsassets.io
    • Triggering command: /home/REDACTED/work/vs-servicehub/vs-servicehub/bin/Microsoft.ServiceHub.Analyzers.Tests/Release/net8.0/Microsoft.ServiceHub.Analyzers.Tests /home/REDACTED/work/vs-servicehub/vs-servicehub/bin/Microsoft.ServiceHub.Analyzers.Tests/Release/net8.0/Microsoft.ServiceHub.Analyzers.Tests --filter-class ISB001DisposeOfProxiesAnalyzerTests --filter-not-trait FailsInCloudTest=true --server dotnettestcli --dotnet-test-pipe /tmp/18d8b871778b44659d0e2c0019646f4d (dns block)
    • Triggering command: /home/REDACTED/work/vs-servicehub/vs-servicehub/bin/Microsoft.ServiceHub.Analyzers.Tests/Release/net8.0/Microsoft.ServiceHub.Analyzers.Tests /home/REDACTED/work/vs-servicehub/vs-servicehub/bin/Microsoft.ServiceHub.Analyzers.Tests/Release/net8.0/Microsoft.ServiceHub.Analyzers.Tests --filter-method *StoredInField_AndDisposedLater --filter-not-trait FailsInCloudTest=true --server dotnettestcli --dotnet-test-pipe /tmp/1f48f344d0174f4f83050de86db321a5 (dns block)
    • Triggering command: /home/REDACTED/work/vs-servicehub/vs-servicehub/bin/Microsoft.ServiceHub.Analyzers.Tests/Release/net8.0/Microsoft.ServiceHub.Analyzers.Tests /home/REDACTED/work/vs-servicehub/vs-servicehub/bin/Microsoft.ServiceHub.Analyzers.Tests/Release/net8.0/Microsoft.ServiceHub.Analyzers.Tests --filter-method *StoredInField_AndDisposedLater --filter-not-trait FailsInCloudTest=true --server dotnettestcli --dotnet-test-pipe /tmp/619d555e41fd4ed595b98ce0a4268c91 (dns block)
  • wlnvsblobprodwus2188.vsblob.vsassets.io
    • Triggering command: /opt/hostedtoolcache/dotnet/dotnet dotnet restore --no-dependencies /home/REDACTED/work/vs-servicehub/.codeql-scratch/dbs/csharp/working/F7D2419353110B6F32053972B32A361D/missingpackages_workingdir --packages /home/REDACTED/work/vs-servicehub/.codeql-scratch/dbs/csharp/working/missingpackages /p:DisableImplicitNuGetFallbackFolder=true --verbosity normal --configfile /home/REDACTED/work/vs-servicehub/.codeql-scratch/dbs/csharp/working/nugetconfig/nuget.config --force (dns block)
  • xofvsblobprodwus2114.vsblob.vsassets.io
    • Triggering command: /opt/hostedtoolcache/dotnet/dotnet dotnet build test/Microsoft.ServiceHub.Analyzers.Tests/Microsoft.ServiceHub.Analyzers.Tests.csproj -c Release (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Copilot AI changed the title Fix ISB001 misfires when proxy is created inside a lambda assigned to a field Fix ISB001 misfires on proxy fields that are disposed Feb 24, 2026
@AArnott AArnott enabled auto-merge (squash) February 24, 2026 20:56
@AArnott
Copy link
Copy Markdown
Member

AArnott commented Feb 24, 2026

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ISB001 misfires on proxy fields that *are* disposed

2 participants