IDisposable и CA2000 предупреждение во время анализа кода VS2010 - PullRequest
3 голосов
/ 24 мая 2011

Мне нужен совет, надеюсь, кто-нибудь мне поможет. У меня есть следующая структура класса (упрощенно):

public class Bar: IDisposable {...}

public abstract class FooBase: IDisposable
{
    Bar bar;
    bool disposed;

    internal FooBase(Bar bar)
    {
        this.bar=bar;
    }

    public void Dispose()
    {
         Dispose(true);
         GC.SupressFinalize(this);

    }

    protected void Dispose(bool disposing)
    {
         if (!this.disposed)
         {
             if (disposing)
             {
                 this.bar.Dispose();
             }

             this.disposed = true;
         }
    }
}

public FooA: Foo {...}
public FooB: Foo {...}

public static class FooProvider
{
    public static FooA GetFooA()
    {
       Bar bar = new Bar();
       ...
       return new FooA(bar);
    }

    public static FooB GetFooB()
    {
        Bar bar = new Bar();
        ...
        return new FooB(bar);
    }

    ...
}

Когда я запускаю Code Analysis для этого, я получаю Warnings CA2000 для всех методов CreateFooX () класса FooProvider. Это предупреждение выдает следующее сообщение:

«Microsoft. Надежность: в методе« FooProvider.GetFooX () »вызовите System.IDisposable.Dispose для объекта« bar »до того, как все ссылки на него выйдут из области видимости."

Microsoft рекомендует никогда не подавлять это предупреждение, но я не совсем уверен, что оно предупреждает о реальной проблеме в коде. Правда, «bar» не удаляется до выхода из области видимости в любом методе «CreateFooX ()», который мы рассматриваем, но ссылка на него находится в объекте «FooX», который в конечном итоге будет удален и, в свою очередь, позаботится об утилизации бар.

Я правильно понимаю, что шаблон Dispose должен работать, и у меня есть какой-то фундаментальный недостаток в коде, или я должен просто отключить это предупреждение?

EDIT

Из-за некоторых комментариев я попытался изменить фабричные методы следующим образом:

public static class FooProvider
{
    public static FooA GetFooA()
    {
       Bar bar = null;

       try
       {
           bar =  new Bar();
           ...
           return new FooA(bar);
       }
       catch
       {
           if (bar != null) bar.Dispose();
           throw;
       }
    }

    ...
}

Но я все еще получаю то же предупреждение. Я предполагаю, что это просто ложный позитив, и я в безопасности, приняв его.

Спасибо за любой совет.

Ответы [ 4 ]

3 голосов
/ 24 мая 2011

Это типичный ложный положительный результат в части анализа кода. Он действительно не может понять внутреннюю ситуацию вашего кода, поэтому он дает общий ответ. Действуйте с осторожностью, но всякий раз, когда вы проверяете наличие ложного срабатывания, вы можете смело игнорировать его.

2 голосов
/ 24 мая 2011

Это не ложный позитив.Что если исключение выдается после создания Bar, но до его передачи конструктору Foo?Я вижу несколько путей кода, в которых один или несколько объектов не могут быть удалены.

0 голосов
/ 27 мая 2011

По крайней мере, часть этой проблемы на самом деле не является ложным срабатыванием, даже если это не обязательно очень полезное обнаружение проблемы. Чтобы решить оставшуюся проблему вашей отредактированной версии, вам нужно открыть блок try сразу после назначения bar, а не до него. , например :

Bar bar = new Bar();
try
{
    ///...            
    return new FooA(bar);
}
catch
{
    bar.Dispose();
    throw;
}

К сожалению, после внесения этого изменения вы все равно получите нарушение CA2000, что, вероятно, будет ложным срабатыванием. Это потому, что правило не проверяет, помещаете ли вы bar в состояние только что созданного FooA. Если оно входит в состояние в FooA, вы можете безопасно создать подавление для нарушения. Однако, если он не перейдет в состояние в FooA, вы должны поместить его в предложение finally вместо предложения catch.

0 голосов
/ 24 мая 2011

Твой одноразовый рисунок мне кажется немного странным. Я не думаю, что вы должны вызывать bar.Despose в классе FooBase. Для обеспечения безопасности объектов, которые вы утилизируете, и возможности вызова safly несколько раз, я бы рекомендовал этот подход.

  private bool _disposed;
  public void Dispose()
  {
     Dispose( true );
     GC.SuppressFinalize( this );
  }

  protected virtual void Dispose( bool disposing )
  {
     if ( disposing )
     {
        if ( !_disposed )
        {
           if ( Bar != null )
           {
              Bar.Dispose();
           }

           _disposed = true;
        }
     }
  }

Что касается ошибки, я думаю, что это должно заботиться о предупреждении статического анализа. Я реализовал ваш код следующим образом в тестовом проекте, включил все предупреждения статического анализа без проблем с предупреждениями.

public class Bar : IDisposable
{
  private bool _disposed;
  public void Dispose()
  {
     Dispose( true );
     GC.SuppressFinalize( this );
  }

  protected virtual void Dispose( bool disposing )
  {
     if ( disposing )
     {
        if ( !_disposed )
        {
           _disposed = true;
        }
     }
  }
}

public abstract class FooBase : IDisposable
{
  public Bar Bar
  {
     get;
     set;
  }

  internal FooBase( Bar bar )
  {
     Bar = bar;
  }

  private bool _disposed;
  public void Dispose()
  {
     Dispose( true );
     GC.SuppressFinalize( this );
  }

  protected virtual void Dispose( bool disposing )
  {
     if ( disposing )
     {
        if ( !_disposed )
        {
           if ( Bar != null )
           {
              Bar.Dispose();
           }

           _disposed = true;
        }
     }
  }
}

public class FooA : FooBase
{
  public FooA( Bar bar )
     : base( bar )
  {
  }
}

public static class FooProvider
{
  public static FooA GetFooA()
  {
     Bar bar;
     using ( bar = new Bar() )
     {
        return new FooA( bar );
     }
  }
}

[TestClass]
public class UnitTest1
{
  [TestMethod]
  public void StaticAnalysisTest()
  {
     Assert.IsNotNull( FooProvider.GetFooA().Bar );
  }
}

Надеюсь, это полезно.

...