0

I have an IDisposable object that can be accessed between multiple threads. I am trying to figure out a way to track that the object is "in use" before performing any clean up. In other words, I need to keep some sort of reference count to indicate that there are running methods (which are decremented when they complete) so that the Dispose method wouldn't continue until they are all complete (or after some timeout has passed). But I also need to make sure that once Dispose has been entered that any future calls to Method fail.

Something like:

class MyObject : IDisposable
{
    private long _counter;
    private bool _stopping;
    private IDisposable _someResource;
    public void Method()
    {
        if (_stopping)
            throw new InvalidOperationException();

        Interlocked.Increment(ref _counter);
        try
        {
            // do some work             
        }
        finally
        {
            Interlocked.Decrement(ref _counter);
        }
    }

    public void Dispose()
    {
        var timeout = DateTime.Now.Add(TimeSpan.FromSeconds(15));
        while ( DateTime.Now < timeout && Volatile.Read(ref _counter) > 0)
        {
            // wait
            // Thread.Sleep(10) or something
        }

        _stopping = true;
        //perform clean up
        _someResource.Dispose();        
    }
}

However this won't work because Method() may be called again and _stopping hasn't been set but the Dispose would continue, invalidating everything else.

Is there a specific pattern that I can use here or perhaps some framework classes that can be used to solve this? Basically some two-way signal telling Dispose that nothing is in process so its good-to-go while signalling Method that it should fail.

I found CountdownEvent but I'm not sure how I can use it here. This answer shows an example CountdownLatch but it doesn't prevent new work from being requested.

Community
  • 1
  • 1
pinkfloydx33
  • 11,863
  • 3
  • 46
  • 63
  • 1
    Why would you make your *object* responsible for this, and not your *clients*? If they need safe concurrent access, let them lock, like the rest of the world does. Although a thread-safe `Dispose` is useful, this can be achieved with a simple `Interlocked.CompareExchange`. Making the disposing thread-safe with regards to *other* operations is unusual. The usual approach is to signal the object as unavailable for any *future* operation from the moment `Dispose` is called, and not try to get fancy with any work still in progress. – Jeroen Mostert Oct 28 '16 at 14:31
  • "[...]`_stopping` hasn't been set[...]" Why don't you assign it a value before your waiting loop ? – Irwene Oct 28 '16 at 14:38
  • Thought about setting it at the start after I wrote the above but figured it'd fall into the same trap. As to why? Because I inherited it like this and I'm trying to see what I can do to fix it. – pinkfloydx33 Oct 28 '16 at 14:48
  • Well, if you can't make your clients lock, you can still trivially do it yourself. Mutual exclusion will solve most any threading problem, and this one's no exception. Doing it safely will get messy, though. Is `Method` already thread safe somehow or is that another accident waiting to happen? – Jeroen Mostert Oct 28 '16 at 15:07
  • `Method` itself is already thread safe. – pinkfloydx33 Oct 28 '16 at 15:21
  • When you start reference-counting then you are well beyond what's reasonable to make IDisposable work. Time to take advantage of the contract, calling Dispose() is *optional*. Don't panic. – Hans Passant Oct 28 '16 at 16:38

1 Answers1

0

No. Just say no to the Dispose method being responsible for "tracking" all instances consuming the class. The consumers of the class need to handle their clean-up and properly dispose.

If Dispose can be called multiple times by multiple classes, then you've just found a code smell. Only one class should being calling the dispose method; and that class should be tracking the consumers of the class with the Dispose method.

A common pattern for preventing dispose from being called multiple times is to set up a flag, similar to what you have now. Use the ObjectDisposedException as needed:

private bool _zombified;

public void Method()
{
    if (_zombified)
        throw new ObjectDisposedException();

    // method's logic
}


public void Dispose
{
    if(_zombified)
    {
        return;
    }

    _zombified = true;

    // perform clean-up
}
Metro Smurf
  • 37,266
  • 20
  • 108
  • 140