Better error when disposed resources are accessed

[mcon]

In C# it's pretty easy to accidentally dispose of a resource whilst it's still being used by a task - if you end up reading a file using ParquetSharp after the ParquetFileReader has been disposed then in stead of getting a (more useful) ObjectDisposedException, the library in stead returns incorrect data.

An explicit version of what I ended up doing (yes, looks very silly when made explicit) was:

var file = new ParquetFileReader("myParquetFile");

// Assuming a row group exists
var groupReader = file.RowGroup(0);

file.Dispose()

// Assuming the group has at least one column
groupReader.Column(0); // Throws exception claiming no columns exist

This issue covers having the reader APIs throw ObjectDisposedException to make it clearer to the user how they've misused the library. There's a non-zero chance I might have time to produce a PR for this one.

[mfkl]

This issue covers having the reader APIs throw ObjectDisposedException to make it clearer to the user how they've misused the library.

Yes, I guess this sounds reasonable. This will break the public API of course. Also this test would need to be changed

[Test]
public static void TestDisposedAccess()
{
    using var buffer = new ResizableBuffer();

    // Write our expected columns to the parquet in-memory file.
    using var outStream = new BufferOutputStream(buffer);
    using var fileWriter = new ParquetFileWriter(outStream, new Column[] {new Column<int>("Index")});

    fileWriter.Dispose();

    var exception = Assert.Throws<NullReferenceException>(() => fileWriter.AppendRowGroup());
    Assert.AreEqual("null native handle", exception?.Message);
}

There's a non-zero chance I might have time to produce a PR for this one.

👍

[GPSnoopy]

Some of the classes like ColumWriter, ColumnReader, RowGroupWriter and RowGroupReader have now an internal reference to their parent (i.e. creator) instance. AFAIK this was introduced by the LogicalTypeFactory PR.

This should make checking for correct disposal a lot easier, since now the parent instances can have an internal IsDisposed flag that can be checked when these classes are themselves disposed.