Multithreading memory leak

  •   Posted in:
  • .NET

Recently I was investigating memory leak in one of our web sites. The same story, site consuming more and more memory until IIS finally recycles it.

As usually, executed two standard commands:
.cordll -ve -u -l
!DumpHeap -stat

And then I found our class Xyz at about 10th place from bottom. Interesting. Then I executed following command:
!DumpHeap -short Namespace.Xyz

And immediately pressed Ctrl+Break to have only few hundred of them and not all of them. And then I executed this command to find how is holding that reference:
!GCRoot <address of Namespace.Xyz object>

From output I found some singleton class with ConcurrentDictionary<Guid, List< Namespace.Xyz>>. Checking few other GCRoots from top, middle and end of Xyz list and they all have the same picture. Ok time to check where Xyz is added to and removed from this dictionary. Simplified version of code looks like this:
 

internal class Singleton
{
    private readonly ConcurrentDictionary<Guid, List<Xyz>> registrations;
        
    public void Register(Guid regoId, Xyz xyz)
    {
        if (xyz == null) return;

        registrations.AddOrUpdate(
            regoId,
            registration => new List<Xyz> { xyz },
            (registration, prev) =>
            {
                if (prev == null)
                    prev = new List<Xyz> { xyz };
                else
                    prev.Add(xyz);

                return prev;
            });
    }

    public void Unregister(Guid regoId, Xyz xyz)
    {
        registrations.AddOrUpdate(
            regoId,
            (List<Xyz>)null,
            (registration, prev) =>
            {
                prev?.RemoveAll(next => xyz.Id == next.Id);

                return prev;
            });
    }
}

I checked it and everything looks correct to me. And my current theory that time was that Xyz is registered but not removed. But fortunately, for me, Xyz is also added to and removed from another list at the same time. I checked that list and I found that it is pretty much empty. And I had to dismiss mt theory.

Then I reviewed code again. After some though I started to think even ConcurrentDictionary is thread safe, its list is not thread safe. I checked source code of ConcurrentDictionary and found that there are no locks held during AddOrUpdate. And as result updating List from different threads will be definitely a problem. But then I started to think what kind of problem. What could happen to List to make it leak memory? After some thought I decided to write simple test that simulate registering and unregistering Xyz.  And I have to admit that first test worked without any problem because each thread generated different Guid. After I did realize it, I rewrote code to use fixed array of pre-generated Guids by all threads. And application immediately crashed with Null Reference Exception in prev?.RemoveAll. After checking variables, I found that next is null. But how? Register checks for null, Add will add to the end of the list and RemoveAll will remove items from list. How it is possible to get null? But I after checking source code of RemoveAll I instantly found answer. When two thread modifying size at the same time it is very possible that there will be nulls.

And then everything starting to fit to place. As soon as there is null in the list it is not possible to remove any items after that as exception was raised when RemoveAll reached null. And Xyz is quite big object and leaking of it quickly exhaust all available memory.

Then I started writing document that explaining my research and then I found quite a lot of System.NullReferenceException. In fact, their number was close to numbers of Xyz objects. And I decided to check call stack of these exceptions. It is possible to see it by executing this command:
!PrintException <object address>

And my satisfaction call stack of that exception was exactly the same place as in my test application. Bingo!

As conclusion, multithreading is always tricky. Question everything. There is good chance that you will find something anyway. And remember that ConcurrentDictionary guarantee thread safety of adding, updating, and removing of key and values. But it does not guarantee any thread safety when you change key or value even you do it in some callbacks that called from ConcurrentDictionary.

I hope it helps someone.