When you access your CloseableThreadLocal from multiple threads, capturedSlots will only be the slots instance from the last thread. Thus on finalizing, all other threads will not be 'closed'.

Since the whole purpose of a *ThreadLocal is to use it from multiple threads I'd say the scenario isn't that rare.

There could be some racing while building the slots and setting the slots more than one time and only the last one will win and it will be that one that is going to be captured.

if (slots == null)

            slots = new Dictionary

<object,> ();

we could do something like this:

if(slots == null)

{

 lock(syncObject)

 {

      if(slots == null)

      {

           slots = new Dictionary

<object,> ();

      }

 }

}

And there is a race condition between the finalizer thread and other threads banging on that dictionary.

@Rodrigo

__slots is a [ThreadStatic] field. Every thread gets it's own version of that field, which means you won't need locking for setting __slots .

@Patrick Huizinga

UPS! You're right. I've missed that

What about instancing the threadstatic static dictionary in the CloseableThreadLocal ctor to capture the thread the instance of CloseableThreadLocal was created on?

Ori, it's not about which thread CloseableThreadLocal was created on, it's about all the threads that use the same instance ctl.

So basically you keep a set of dictionaries on each instance, containing every dictionary they've been inserted into.

Better yet, keep a weak reference to those dictionaries. I assume that [ThreadLocal] is smart enough to prevent the GC from collecting any of the referenced items while the thread is alive, and to stop preventing the GC from collecting those items when the thread dies. If you use a strong reference, a dictionary for a thread that has died will stay in memory as long as at least one ClosableThreadLocal which has been accessed from that thread is alive, just to do unnecessary cleanup.

If you have concurrent collection enabled, you have problems. Your destructors might run while another thread is modifying the dictionary. You need a dictionary that can handle updates from multiple threads correctly without locking. Perhaps it would mediate updates through a dictionary of Thread => Queue, and you automatically drop updates marked with threads other than explicitly registered threads (where registering a thread requires locking).

Ugh, this is spiraling out of control... there has to be another approach that will work better.

Cant see how would this work without ordinary locking - this scenario always leads to the point, where you have to access some sort of thread-shared resource.

ThreadStatic is cleaned up after thread dies, so it is no problem with it not being cleaned up (there is only one reference to it). When thread dies, all data within it's scope gets cleaned up nicely.

The problem shows up, when you create lots of objects in the scope of single thread, because their values are stuck inside thread-static dictionary until the end of this thread - if it's long enough or object quantity is high enough - you have a problem.

I also don't know how WeakReference can help here, because all it can do is set it's target (stored) to null when GC "has a good mood" for that. So you end up with locking :-(

This is as far as I could get, it's ugly when you create lots of objects, that are dropped immediately, because it slows down finalizer on static locking.... It also keeps all thread-static dictionaries referenced inside instances ... but it at least makes it possible to get to the point, where all is cleaned up:

public class CloseableThreadLocal

{

    [ThreadStatic]

    public static Dictionary

<object,> slots;

    private static object knownSlotsSyncRoot = new object();


    private readonly object holder = new object();


    private readonly List

<dictionary<object,>

knownSlots =

        new List

<dictionary<object,>

();

    private Dictionary

<object,> Slots

    {

        get

        {

            if (slots == null)

            {

                lock (knownSlotsSyncRoot)

                {

                    slots = new Dictionary

<object,> ();

                    knownSlots.Add(slots);

                }

            }


            return slots;

        }

    }



    public /*protected internal*/ virtual Object InitialValue()

    {

        return null;

    }


    public virtual Object Get()

    {

        object val;


        if (Slots.TryGetValue(holder, out val))

        {

            return val;

        }

        val = InitialValue();

        Set(val);

        return val;

    }


    public virtual void Set(object val)

    {

        Slots[holder] = val;

    }


    public virtual void Close()

    {

        GC.SuppressFinalize(this);

        lock (knownSlotsSyncRoot)

        {

            foreach (var slot in knownSlots)

            {

                slot.Remove(holder);

            }


            slots = null;

        }


    }


    ~CloseableThreadLocal()

    {

        lock (knownSlotsSyncRoot)

        {

            foreach (var slot in knownSlots)

            {

                slot.Remove(holder);

            }

        }

    }

}