How to dispose objects having asynchronous methods called?

Question

I have this object PreloadClient which implements IDisposable, I want to dispose it, but after the asynchronous methods finish their call... which is not happening

    private void Preload(SlideHandler slide)
    {
        using(PreloadClient client = new PreloadClient())
        {                 
             client.PreloadCompleted += client_PreloadCompleted;
             client.Preload(slide);
        }
        // Here client is disposed immediately
    }
    private void client_PreloadCompleted(object sender, SlidePreloadCompletedEventArgs e)
    {
     // this is method is called after a while, 
     // but errors are thrown when trying to access object state (fields, properties)
    }

So, any ideas or work arounds ??

vgru · Accepted Answer · 2013-03-27T13:48:30.593

You shouldn't use the using construct, but rather dispose your objects when they are no longer needed:

// keep a list of strong references to avoid garbage collection,
// and dispose them all in case we're disposing the encapsulating object
private readonly List<PreloadClient> _activeClients = new List<PreloadClient>();
private void Preload(SlideHandler slide)
{
    PreloadClient client = new PreloadClient();
    _activeClients.Add(client);
    client.PreloadCompleted += client_PreloadCompleted;
    client.Preload(slide);
}

private void client_PreloadCompleted(object sender,
     SlidePreloadCompletedEventArgs e)
{
    PreloadClient client = sender as PreloadClient;

    // do stuff

    client.PreloadCompleted -= client_PreloadCompleted;
    client.Dispose();
    _activeClients.Remove(client);
}

in this case, you have to dispose all clients when disposing the main class:

protected override Dispose(bool disposing)
{
    foreach (PreloadClient client in _activeClients)
    { 
        client.PreloadCompleted -= client_PreloadCompleted;
        client.Dispose();
    }
    _activeClients.Clear();
    base.Dispose(disposing);
}

Note that this implementation is not thread safe
- Access to the _activeClients list must be made thread-safe, as your PreloadCompleted method is called from a different thread
- Your containing object may be disposed before a client fires the event. In that case "do stuff" should do nothing, so this is another thing you should take care of.
- It might be a good idea to use a try/finally block inside your event handler, to make sure that the object gets disposed in all cases

A pattern that would seem applicable here, and which I'd like to see used more often, would be to have the client include an `IsDisposed` property, have `_activeClients` hold a list of `WeakReference` to clients, and have the act of adding an item to `_activeClients` check a couple items to see if either the `WeakReference` has died or the target is disposed, "swapping" it with the next item if so. That would avoid any need to lock the list in the asynchronous callback, and also replace the O(N) cost of removing each item from the list with an O(1) extra cost when the item is added. — supercat, Mar 27 '13 at 20:34
@niico You can only use `using` within a single block, i.e. a single clause. This means the object will be disposed once the `using` block ends. In this case, OP called an asynchronous `Preload` method, so s/he wanted to delay disposing until all async work has finished. — vgru, Jan 10 '15 at 13:12

score 3 · Answer 2 · answered Jun 10 '09 at 11:13

3

Why not dispose the client in the callback?

answered Jun 10 '09 at 11:13

almog.ori

7,839
1
35
49

1

If you have 2 or more events ... which one you pick to dispose :) ? – Jalal El-Shaer Jun 10 '09 at 11:33
@jalchr: If you implemented "Dispose" *correctly*, calling dispose more than once does not affect the outcome no matter how many time "Dispose" is called. – dance2die Jun 10 '09 at 12:52
1

@jalchr: If you need to dispose an object when the last callback fires, use some lock- or interlock-protected field to keep track of how many callbacks are outstanding, and have each callback check whether it's the last one and dispose if so. – supercat Feb 20 '11 at 21:10

score 1 · Answer 3 · answered Jun 10 '09 at 11:12

1

I have a few ideas:

change your architecture.
dispose in the handler
use EventWaitHandle

answered Jun 10 '09 at 11:12

Krzysztof Kozmic

27,267
12
73
115

check this out about EventWaitHandle : http://wekempf.spaces.live.com/Blog/cns!D18C3EC06EA971CF!672.entry?sa=571623833 – Jalal El-Shaer Jun 10 '09 at 11:33

score 0 · Answer 4 · answered Jun 10 '09 at 11:14

If there are event handlers being registered, you can't really dispose the object while there are events that could be called on it. Your best bet is to make the containing class disposable & store the client in a class variable, to be disposed when the containing class is.

Something like

class ContainingClass : IDisposable
{
    private PreloadClient m_Client;

    private void Preload(SlideHandler slide)
    {
         m_Client = new PreloadClient())

         m_Client.PreloadCompleted += client_PreloadCompleted;
         m_Client.Preload(slide);

    }
    private void client_PreloadCompleted(object sender, SlidePreloadCompletedEventArgs e)
    {
    }

    public void Dispose()
    {
        if (m_Client != null)
            m_Client.Dispose();
    }
}

Bad idea. Now you tied lifetime of containingclass with lifetime of client. what If Preload is called more than once? You won't dispose client. What if you call dispose on container before async operation completes? You'll most likely get ugly exception — Krzysztof Kozmic, Jun 10 '09 at 11:25

score 0 · Answer 5 · answered Jun 10 '09 at 11:15

Well, disposing an object is used to kill resources that you don't want held until the GC (eventually) comes and collects your object. Does your dispose method kill anything you need in client_PreloadCompleted?

You could have the object dispose itself, when all expected callbacks have happened: Keep a "reference counter" for each callback you are expecting and decrement that on each callback that happens - check for null at end of callback handler and dispose if so.

Other workaround: Don't worry about IDisposable. GC will collect your object. You probably don't want a callback handler (that might not be fired) to have critical state. It (the callback) should just open any resources it needs when it is called and close them then.

score 0 · Answer 6 · answered Jun 10 '09 at 11:21

0

Asynchronous waits and deterministic disposal don't mix very well. If you can find a way of splitting the code such that the disposable stuff goes in one class and the events go in another, that would make everything simpler.

answered Jun 10 '09 at 11:21

Christian Hayter

30,581
6
72
99

Could you please elaborate more ... what do you mean by putting events in another class ... ? – Jalal El-Shaer Jun 10 '09 at 11:43
Take a hypothetical class that holds state which requires deterministic cleanup. That class is a good candidate for IDisposable. However, if you call an asynchronous method on it, you then have to wait for the asynchronous method to complete before you can dispose it. Why bother? You might as well make it a synchronous method and dispose the object when the method completes. You have accomplished the same thing in the same amount of time. – Christian Hayter Jun 11 '09 at 07:26

score 0 · Answer 7 · answered Jun 10 '09 at 11:30

Why not disposing in the client_PreloadCompleted method? Similar to what thecoop offered, just with the Dispose call inside the above method, after you have accessed all the needed data from inside the client object.

Edit: I think that is what orialmog offered as well.

How to dispose objects having asynchronous methods called?

7 Answers7

Linked