views:

334

answers:

1

So I'm working on my DI/IoC Container OpenNETCF.IoC and I've got a (reasonable) feature request to add some form of lifecycle management for IDisposable items in the container collections.

My current thinking is that, since I can't query an object to see if it's been disposed, and I can't get an event for when it's been disposed, that I have to create some form of wrapper for objects that a developer wants the framework to manage.

Right now objects can be added with AddNew (for simplicity sake we'll assume there's only one overload and there is no Add):

public TTypeToBuild AddNew<TTypeToBuild>() { ... }

What I'm considering is adding a new method (well group of them, but you get the picture):

public DisposableWrappedObject<IDisposable> AddNewDisposable<TTypeToBuild>()
    where TTypeToBuild : class, IDisposable
{
    ...
}

Where the DisposableWrappedObject looks like this:

public class DisposableWrappedObject<T>
    where T : class, IDisposable
{
    public bool Disposed { get; private set; }
    public T Instance { get; private set; }

    internal event EventHandler<GenericEventArgs<IDisposable>> Disposing;

    internal DisposableWrappedObject(T disposableObject)
    {
        if (disposableObject == null) throw new ArgumentNullException();

        Instance = disposableObject;
    }

    ~DisposableWrappedObject()
    {
        Dispose(false);
    }

    public void Dispose()
    {
        Dispose(true);
    }

    protected virtual void Dispose(bool disposing)
    {
        lock(this)
        {
            if(Disposed) return;

            EventHandler<GenericEventArgs<IDisposable>> handler = Disposing;
            if(handler != null)
            {
                Disposing(this, new GenericEventArgs<IDisposable>(Instance));
            }

            Instance.Dispose();

            Disposed = true;
        }
    }
}

Now, when an items gets added to the container via AddNewDIsposable, an eventhandler is also added so that when it gets Disposed (via the wrapper) the framework removes it from the underlying collection.

I actually have this implemented and it's passing the unit tests, but I'm looking for opinions on where this might be broken, or how it might be made more "friendly" to the consuming developer.

EDIT 1

Since there was a question on how the Disposing event is used, here's some code (trimmed to what's important):

private object AddNew(Type typeToBuild, string id, bool wrapDisposables)
{
    ....

    object instance = ObjectFactory.CreateObject(typeToBuild, m_root);

    if ((wrapDisposables) && (instance is IDisposable))
    {
        DisposableWrappedObject<IDisposable> dispInstance = new
               DisposableWrappedObject<IDisposable>(instance as IDisposable);
        dispInstance.Disposing += new 
               EventHandler<GenericEventArgs<IDisposable>>(DisposableItemHandler);
        Add(dispInstance as TItem, id, expectNullId);
        instance = dispInstance;
    }

    ....

    return instance;
}

private void DisposableItemHandler(object sender, GenericEventArgs<IDisposable> e)
{
    var key = m_items.FirstOrDefault(i => i.Value == sender).Key;
    if(key == null) return;
    m_items.Remove(key);
}
+3  A: 

Maybe I'm missing something, but why add new methods to the API? When an object is added to the container, you could as-cast to check if it's IDisposable and handle it appropriately if so.

I'm also wondering if you need the destructor. Presuming the container is IDisposable (like Unity's), you could just implement the Basic Dispose Pattern and save a lot of GC overhead.

Some questions that may be applicable:

TrueWill
Ah, but with an as-cast, then what? I know it's IDisposable and has to be put into a container, but I can't return said container because the AddNew<> method has to return the input type. If I return the object directly as the API wants, they don't know it was wrapped, and then just call Dispose on the instance, not the container, and again I have the problem of holding Disposed objects that never get GC'ed.
ctacke
@ctacke: There are some rules that consumers must follow when they play with IoC: When you hand off the responsibility of *creating* objects to a DI Container, you must hand off *all* lifetime managment, including *destruction* of instances. Thus, it would be an error on the consumer's side if they prematurely dispose of an injected dependency, since that instance may be shared among multiple consumers. I don't think you need to over-complicate your API to safeguard against usage that is just plain wrong.
Mark Seemann