8

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);
}
ctacke
  • 65,117
  • 17
  • 91
  • 151
  • Can we get a fuller description of the specific feature that is being added? What are the use cases that people want this for, is the event handler for you as the (IoC framework) or the end user? Etc.. – Quibblesome Oct 13 '09 at 11:03
  • The use case is to add automated lifecycle management. If you add an IDisposable item to a collection and later call Dispose, it will never actually get cleaned up becasue the container holds a root to the object. The idea is that you can call Dispose on the object without having to go back to the collection to find it, and have that automatically cause removal from teh container collection. The event is used purely internally by the framework (it's even marked as internal so as not to get used outside) and the handler removes it from the collection. – ctacke Oct 13 '09 at 12:50
  • I've updated the question to add the event handling for clarity. – ctacke Oct 13 '09 at 12:56
  • So the consumer has disposed of the object and they want to ensure you don't hold onto it anymore? What happens if they request it again, do you make a new one? In terms of the code the only thing unusual I can notice is that you're playing with the GC thread off the finalizer via the event handler, so be careful with that code! :) – Quibblesome Oct 13 '09 at 14:01
  • @ctacke: "If you add an IDisposable item to a collection and later call Dispose, it will never actually get cleaned up becasue the container holds a root to the object." Holding references only applies to GC. Disposing of unmanaged resources is supposed to be the developer's responsibility. Calling Dispose **will** do the cleanup immediately, regardless of what other references exist. – TrueWill Oct 13 '09 at 17:30
  • Yes, it will clean any *native* resources, but the point is that the object itself, including all of its managed resources, will never be released and that is the root of the problem I'd attempting to solve. If it's something like a UserControl or a Form in a Compact Framework application, this could be a problem. Mobile devices simply don't have the resources to accomodate programmer laziness. You can't just throw everything into the container and not worry about cleaning up after yourself like you can on the desktop. – ctacke Oct 13 '09 at 19:31

1 Answers1

3

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:

Community
  • 1
  • 1
TrueWill
  • 23,842
  • 7
  • 88
  • 133
  • 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 Oct 13 '09 at 04:52
  • 5
    @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 Oct 13 '09 at 11:38