Finalization of ConcurrentBag containing unmanaged objects

Question

I'm having trouble handling Dispose/Finalization correctly with a ConcurrentBag that contains unmanaged objects. Running the below code (usually) produces an ObjectDisposedException (Cannot access a disposed object.Object name: 'The ThreadLocal object has been disposed.'.) on the call to TryTake().

Presumably in this case the garbage collector is destroying the ConcurrentBag before calling A's finalizer. I had thought this would only the case if the ConcurrentBag itself implemented a finalizer. Is it the case that one should never touch managed objects during the finalization path?

class A : IDisposable
{
    private readonly ConcurrentBag<object> _collection = new ConcurrentBag<object>();

    public A(string value)
    {
        if (value == null) throw new ArgumentNullException();
    }

    ~A() 
    {
        Dispose(false);
    }

    public void Dispose() => Dispose(true);

    private void Dispose(bool disposing)
    {
        if (disposing) {}

        object value;
        while (_collection.TryTake(out value))
        {
            // Cleanup value
        }
    }
}

Trigger the exception:

void Main()
{
    var a = new A(null);
}

The following seems to work around this specific issue but I'm unsure if this is safe. Is there a perfectly safe implementation for this scenario?

while (_collection.IsEmpty == false)
{
    object value;
    _collection.TryTake(out value);
    // Cleanup value
}

Answer 1

When code is executing from the finalizer (disposing is false) the only things you are allowed to do is use static methods with no state, variables local to the function, and fields that inherit from CriticalFinalizerObject (unless you are in a finalizer for a CriticalFinalizerObject then you can't use them).

Because ConcurrentBag does not inherit from CriticalFinalizerObject you can not rely on it not being finalized when your own finalizer is run. Both this and the _collection.m_locals variable Sign mentions in his answer both get put in to the finalization queue at the same time when this becomes unreachable. The order the queue is processed is not deterministic.

There is a great article "IDisposable: What Your Mother Never Told You About Resource Deallocation" that goes in depth on what is actually happening when somthing gets finalized and provides some better patterns than the traditional private void Dispose(bool disposing) pattern Microsoft recommends.

Answer 2

The full stack trace of the object disposed exception is

   at System.Threading.ThreadLocal`1.GetValueSlow()
   at System.Threading.ThreadLocal`1.get_Value()
   at System.Collections.Concurrent.ConcurrentBag`1.GetThreadList(Boolean forceCreate)
   at System.Collections.Concurrent.ConcurrentBag`1.TryTakeOrPeek(T& result, Boolean take)
   at System.Collections.Concurrent.ConcurrentBag`1.TryTake(T& result)
   at A.Dispose(Boolean disposing)
   at A.Finalize()

Which means that the disposed object lives inside ConcurrentBag which is odd since ConcurrentBag isn't IDisposable. Digging through the source of ConcurrentBag shows that it has a ThreadLocal which is IDisposable and is used in the GetThreadList method. Weirdly enough if you use a plain old foreach loop you avoid the ThreadLocal and it looks like everything works the way you would expect.

foreach (var value in _collection)
{
   // Cleanup value
}

Despite this digging I don't have an explanation for how the ThreadLocal got disposed.