Search code examples
c#disposeusing

New `using` syntax interaction with long lived references


Here's some example code;

public sealed class HoldingClass : IDisposable
{
  private BinaryReader _binaryReaderField;

  public string GetStringFromReader(int count)
  {
    var data = _binaryReaderField.ReadBytes(count);
    // ... Process data ... //
    return newString;
  }

  public void Dispose()
  {
    _binaryReaderField?.Close();
    _binaryReaderField?.Dispose();
  }
}

// ... somewhere else ... //

public sealed class EntryPoint : IDisposable
{
  public static Start(string filePath)
  {
    using var br = new BinaryReader(File.OpenRead(filePath));
    return new EntryPoint { _holdingClass = new HoldingClass(br) };
  }

  private HoldingClass _holdingClass;

  public string GetString(int count)
  {
    return _holdingClass.GetStringFromReader(count);
  }

  public void Dispose()
  {
    _holdingClass?.Dispose();
  }
}

Please note this is a contrived example to express the behaviour I'm interested in and does not represent any sort of production code.

Ok, so in the above code let's say the application starts up with using var ep = EntryPoint.Start("someFile.txt"); and later on in the method after some other processing occurs, the developer then calls ep.GetString(155);.

What is the behaviour of the BinaryReader using statement?

"Common-sense" says to me that as soon as you leave the scope of the EntryPoint.Start method, the object is disposed. Any attempt to use it later through the HoldingClass instance will result in an ObjectDisposed exception. However, I don't know quite how the new using intelligent behaviour works. Is it done by reference counting? In which case the BinaryReader object would stay alive throughout the life of the HoldingClass instance.

In C++ I would solve this by instantiating a unique pointer and transferring ownership to the new object, but C# doesn't work like that, as far as I know.


Solution

  • The new using syntax is really just syntactic sugar to reduce "noise" from needing to use braces to show the scope the object should be disposed after. There's no "new" heuristics about it. The equivalent old-style for the code you've posted would be:

    public static EntryPoint Start(string filePath)
    {
        using (using var br = new BinaryReader(File.OpenRead(filePath)))
        {
            return new EntryPoint { _holdingClass = new HoldingClass(br) };
        }
    }
    

    In either style, this is a code bug and the holding class would always contain a disposed reference as it is disposed of before the caller ever has access to the EntryPoint instance returned by the Start() method.

    Using old or new syntax, what you're trying to do here isn't an appropriate usage of using, and you should omit it with something like:

    public static EntryPoint Start(string filePath)
    {
        var br = new BinaryReader(File.OpenRead(filePath));
        try
        {
            return new EntryPoint { _holdingClass = new HoldingClass(br) };
        }
        catch (Exception)
        {
            br.Dispose();
            throw;
        }
    }
    

    Here the BinaryReader is disposed of if an exception happens while creating the result, but otherwise you need to pass control of it to your HoldingClass implementation and have both it and EntryPoint implement IDisposable and appropriate dispose of it when they are disposed of.