Wednesday 23 February 2011

IDisposable and Collections

Ah, joy! The .NET collections don't clean up their contained objects if said objects are IDisposable! Boo! So, if the collection contains IDisposable then these objects need to be Disposed individually before the collection itself is discarded. Some swift googling revealed that others had experienced this issue (see stackoverflow, for example).

Snipped from stackoverflow:

public class DisposableList : List, IDisposable where T : IDisposable
{
#region IDisposable Members

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

public virtual void Dispose(bool disposing)
{
if (disposing)
{
foreach (T type in this)
{
try
{
type.Dispose();
}
finally {/* In case of ObjectDisposedException*/}
}
}
}
#endregion
}

Investigation reveals that CSMInstrument implements IDisposable! Bah! How many of them are scattered throughout the code, also being held in collections?

6 comments:

  1. It's almost guaranteed that the reason Sophis say not to use Dispose on any object which is returned from a const pointer is because it's been cached. But this means we have to assume correspondence between the C# method and the C++ one! Nice!

    Also I hadn't realised that there isn't much in the way of reference counting in .NET. Specifically, create a disposable object, two references to it and insert it into a container (which calls Dispose on its contents when it itself is Disposed). Then we are left with dangling references after the container is tidied away. Nice! What we need to counter that is a deep clone, methinks....

    ReplyDelete
  2. Well, Sophis might say we shouldn't Dispose objects which are returned via a const pointer *but* testing doesn't yield any indication why this might be the case.

    Specifically, the below NUnit test succeeds:

    [Test]
    public void DisposeInstrumentTest2()
    {
    OracleConnection connection = OracleUtils.GetConnection();
    if (connection != null)
    {
    try
    {
    List sicovams = new List();
    using (OracleCommand command = connection.CreateCommand())
    {
    command.CommandType = CommandType.Text;
    command.CommandText = "select sicovam from titres where type='A' order by sicovam";
    command.Prepare();

    using (OracleDataReader reader = command.ExecuteReader())
    {
    if (!reader.HasRows)
    {
    throw new Exception("Could not load any instruments from titres: " + command.CommandText);
    }
    while (reader.Read())
    {
    int? instrumentCode = OracleUtils.GetInt32(reader, "SICOVAM");
    if (instrumentCode.HasValue)
    {
    sicovams.Add(instrumentCode.Value);
    }
    else
    throw new Exception("Loaded null rate_code for interpolated rate " + instrumentCode);
    }
    }
    }

    using (DisposableList ll = new DisposableList())
    {
    foreach (int sicovam in sicovams)
    {
    CSMInstrument inst = CSMInstrument.GetInstance(sicovam);
    if (inst != null)
    ll.Add(inst);
    else
    throw new Exception("Cannot recognise instrument " + sicovam);
    }
    Assert.IsTrue(ll.Count == sicovams.Count);
    }

    List lli = new List();
    foreach (int sicovam in sicovams)
    {
    CSMInstrument inst = CSMInstrument.GetInstance(sicovam);
    if (inst != null)
    lli.Add(inst);
    else
    throw new Exception("Cannot load instrument " + sicovam);
    }

    Assert.IsTrue(lli.Count==sicovams.Count);

    IEnumerator iter = lli.GetEnumerator();
    foreach (int sicovam in sicovams)
    {
    Assert.IsTrue(iter.MoveNext());
    Assert.IsTrue(iter.Current.GetCode()==sicovam);
    }
    }
    finally
    {
    OracleUtils.ReturnConnection(connection);
    }
    }
    else throw new Exception("Could not open Oracle connection");
    }

    ReplyDelete
  3. So.... CSMInstrument.GetInstance goes to the database to populate the instrument object cache in Sophis. Calling Dispose seems to have no effect, even with a GC run between disposing of the DisposableList and the creation of a new List.

    We infer from the above that it is safe to call Dispose and that Dispose only cleans the .NET cache, leaving the C++ object cache untouched.

    ReplyDelete
  4. Arrr Jimlad. The CSMTransactionVector implements IEnumerable, but it doesn't work properly! In v5.3.6.x this code fails in the inner loop because the transaction are all null. Using foreach appears to delete the current element. Instead of using foreach, advance through the vector using operator[].

    using (CSMTransactionVector transactions = new CSMTransactionVector())
    {
    position.GetTransactions(transactions);

    int count = transactions.Count;
    //
    // Dodgy Sophis - don't use foreach on the transaction vector
    //
    foreach (CSMTransaction transaction in transactions)
    {
    Assert.NotNull(transaction);
    }

    Assert.True(count == transactions.Count);
    }

    ReplyDelete
  5. Duh! It fails in the Count with an Access Violation because the enumerator deletes the transaction behind when MoveNext is called.

    Get it right!!

    ReplyDelete
  6. CSMTransactionVector takes ownership if the elements!! When it is Disposed then all of the contained elements are reset!

    ReplyDelete