ChallengeYour own ThreadLocal

time to read 4 min | 767 words

I found a bug in Lucene.NET that resulted in a memory leak in RavenDB. Take a look here (I have simplified the code to some extent, but the same spirit remains):

public class CloseableThreadLocal
{
    [ThreadStatic] private static Dictionary<object, object> slots;

    public static Dictionary<object, object> Slots
    {
        get { return slots ?? (slots = new Dictionary<object, object>()); }
    }

    public /*protected internal*/ virtual Object InitialValue()
    {
        return null;
    }

    public virtual Object Get()
    {
        object val;

        if (Slots.TryGetValue(this, out val))
        {
            return val;
        }
        val = InitialValue();
        Set(val);
        return val;
    }

    public virtual void Set(object val)
    {
        Slots[this] = val;
    }

    public virtual void Close()
    {
        if (slots != null)// intentionally using the field here, to avoid creating the instance
            slots.Remove(this);
    }
}

As you can imagine, this is a fairly elegant way of doing this (please note, .NET 4.0 have the ThreadLocal class, which I strongly recommend using). But it has one very serious flaw. It you don’t close the instance, you are going to leak some memory. As you can imagine, that is a pretty bad thing to do.

In general, I consider such designs as pretty bad bugs when writing managed code. We have the GC for a reason, and writing code that forces the user to manually manage memory is BAD for you. Here is an example showing the problem:

class Program
{
    static void Main(string[] args)
    {
        UseThreadLocal();
        GC.Collect(2);
GC.WaitForPendingFinalizers();
Console.WriteLine(CloseableThreadLocal.slots.Count); } private static void UseThreadLocal() { var tl = new CloseableThreadLocal(); tl.Set("hello there"); Console.WriteLine(tl.Get()); } }

This will show that after the UseThreadLocal() run and we force full collection, the value is still there.

Without using the builtin ThreadLocal, can you figure out a way to solve this?

Points goes to whoever does this with the minimum amount of changes to the code.

More posts in "Challenge" series:

  1. (03 Jan 2020) Spot the bug in the stream–answer
  2. (15 Feb 2010) Where is the optimization?