BADF00D / DisposableFixer

This is a Visual Studio Extension and NuGet package that should identify and fix problems as memleaks while using IDisposables.
Other
35 stars 7 forks source link

Close() corresponding Dispose() #70

Closed leluc42 closed 6 years ago

leluc42 commented 6 years ago

Prerequisites

Description

If i have an object, like an SerialPort, which implements an Close()-method, there is the Error DF0010 (Local variable is not disposed) to. But the Close()-method normally is calling the Dispose()-method. Definitely the original classes from the .Net Framework implement this behaviour.

In fact, it should not be an Error, if an object's Close()-method is called.

Source Code

SerialPort port= new SerialPort(); port.Close();

BADF00D commented 6 years ago

Hi, first of all, thanks for reporting this error.

I just disassembled SerialPort and could confirm what you are saying. Its pretty uncommon that Dispose is called by another method within the same class. Usually a method like Close calls Dispose, e.g. in Stream and nearly everything that is derived from Stream:

public void Dispose() {
    /* These are correct, but we'd have to fix PipeStream & NetworkStream very carefully.
    Contract.Ensures(CanRead == false);
    Contract.Ensures(CanWrite == false);
    Contract.Ensures(CanSeek == false);
    */
    Close();
}

I'm still thinking what is the best way to handle this special situation...

BADF00D commented 6 years ago

Will be fixed in version 0.35

xperiandri commented 6 years ago

What about streams and ADO.NET classes that use Close? Where Dispose calls Close and may be exposed via interface only

BADF00D commented 6 years ago

@xperiandri Do you have any specific example code that does not work as you expected it to work?

xperiandri commented 6 years ago
SqlConnection connection = null;
SqlCommand command = null;
SqlDataReader reader = null;
try
{
    connection = new SqlConnection(connectionString);
    await connection.OpenAsync();

    command = connection.CreateCommand();
    command.Connection = connection;
    command.CommandText = "";
    command.CommandType = CommandType.Text;
    stopWatch.Start();
    reader = await command.ExecuteReaderAsync(); // << warning!!!
    stopWatch.Stop();
    LoggerHelper.LogStress("LoadCustomerInformations", "SELECT [Name]", stopWatch.Elapsed, "");
    stopWatch.Restart();
    while (await reader.ReadAsync())
    {
        customerInformations.Add(new KeyValuePair<string, string>((string)reader["Name"], (string)reader["ConnectionString"]));
    }
    LoggerHelper.LogStress("LoadCustomerInformations", null, stopWatch.Elapsed, "");
}
finally
{
    reader?.Close();
    connection?.Dispose();
    command?.Dispose();
}
BADF00D commented 6 years ago

Hi, I just disassembled the SqlDataReader and can now confirm, that Close of base class DbDataReader calls Close in Dispose:

/// <summary>Releases the managed resources used by the <see cref="T:System.Data.Common.DbDataReader" /> and optionally releases the unmanaged resources.</summary>
    /// <param name="disposing">
    /// <see langword="true" /> to release managed and unmanaged resources; <see langword="false" /> to release only unmanaged resources.</param>
    protected virtual void Dispose(bool disposing)
    {
      if (!disposing)
        return;
      this.Close();
    }

So a call to Close on all DbDataReader can be seen as a call to Dispose.

I created a new ticket #83 for this additional classes.

xperiandri commented 6 years ago

Thanks