2

I am writing to some tables in a database in C# using a 3rd party database library: Esri file-geodatabase-api. For each table type I have implemented a writer class. I pass a reference to a database object to each writer. Each writer then opens its own table in the database.

Both the database and the table class (3rd party) have Close() methods. They also implement IDisposable!? According to Esri samples I should call Close().

Here is a simplified version of my code. Am I doing things right here? Coming from C++ the automatic garbage collection confuses me.

public class TableWriter : IDisposable
{
    // Geodatabase is a 3rd party class that implements  IDisposable and has Close() method: 
    private Geodatabase geoDatabase_;
    // Table is a 3rd party class that implements  IDisposable and has Close() method: 
    protected Table table_; 
    private string tableName_;
    private bool disposing_;

    public TableWriter(Geodatabase geoDatabase, string tableName)
    {
        geoDatabase_ = geoDatabase;
        tableName_ = tableName;
        disposing_ = false;
    }

    // Constructors of subclasses calls this:
    public void CreateTable(List<FieldDef> fieldDefList)
    {
        table_ = geoDatabase_.CreateTable(tableName_, fieldDefList.ToArray(), "");
    }

    public void Dispose()
    {
        Dispose(true);
        GC.SuppressFinalize(this);  
    }
    protected virtual void Dispose(bool disposing)
    {
        if (disposed_)
        {
            return;
        }
        if(disposing) // called by user and not garbage collector
        {
            table_.Close();
            table_.Dispose(); // can/should I do both Close and Dispose?
            geoDatabase_ = null; // decrement reference count?
        }
        disposed_ = true;
    } 
    // subclasses override this (they cast TableRow to a subtype):     
    public virtual void Write(TableRow tableRow){}
}

The export to database is triggered from a gui. I got a crash when I ran this export many times in sequence. I have since rewritten my code using the Dispose pattern and have not yet managed to trigger the crash. The crash seemed a bit random. Sometimes I could run it 5 times in row without a crash other times 15 times in row without a crash. The stack of the crash is this:

[19352] Exception Details:  Cannot access a disposed object. Object name: 'Geodatabase'. 
[19352]    at Esri.FileGDB.Geodatabase.get_NativeGeodatabase() 
[19352]    at Esri.FileGDB.Table.Shutdown() 
[19352]    at Esri.FileGDB.Table.Dispose(Boolean A_0)

To me it seems that the garbage collector called automatic code that double deleted a table in one of the writers.

Andy
  • 3,251
  • 4
  • 32
  • 53
  • 1
    You might be able to suppress the finalization of the table object by using GC.SuppressFinalize (https://learn.microsoft.com/en-us/dotnet/api/system.gc.suppressfinalize?view=netframework-4.8). – Christiaan Nieuwlaat Aug 22 '19 at 08:58
  • 2
    Do you have an asynchronous event handler that writes to the database? If so, then this is a classic race condition problem, because the event could be "in flight" while Geodatabase is being disposed. – Matthew Watson Aug 22 '19 at 09:01
  • 2
    Please see https://stackoverflow.com/a/898867/11683 for how to properly implement the Disposable pattern (three levels depending on what your object has to manage). In any case your code should be written in such way that calling `Dispose` on an already disposed object is safe and does nothing. And it does not look like the classes you've shown have any unmanaged resources. – GSerg Aug 22 '19 at 09:09
  • So I should call GC.SuppressFinalize() in my Dispose method? How do I release the tableName_ string then? tableName_ = null? Likewise I can release a "reference" to the geoDatabase_ by geoDatabase_ = null? – Andy Aug 22 '19 at 10:50
  • Thank you all for your help! I have rewritten my question using the Disposable pattern. Still not sure if the actual dispose code is correct though. – Andy Aug 22 '19 at 12:39

1 Answers1

1

First, make your class sealed. The rules for implementing IDisposable are considerably simpler for sealed types compared to unsealed types (e.g. you don't need the Boolean disposing parameter).

Then, implement Dispose like so:

public void Dispose()
{
    this.table_.Close();
    GC.SuppressFinalize(this);
}

While the root cause of your problem is the fact that your Esri.FileGDB library's Dispose method is not idempotent (i.e. a Dispose() call should never throw ObjectDisposedException - it should always be safe to call a single instance's Dispose() multiple times).

I'm assuming that your database library also exposes finalizers (destructors) that incorrectly call .Dispose() or .Close() methods which then cause the ObjectDisposedException.

Dai
  • 141,631
  • 28
  • 261
  • 374
  • But I have subclasses implementing TableWriter? Eg. PointsWriter : TableWriter, LinesWriter : TableWriter etc. – Andy Aug 22 '19 at 12:15
  • I am somewhat surprised by the complexities of the Dispose pattern. I think this would be easier in C++. Am I correct in understanding that it should not be this hard unless you write a library that uses unmanaged resources. However since Esri have not done this completely correct they have pushed the task of making their Dispose method only be called once over to the users of the library? – Andy Aug 22 '19 at 12:57