A Cool / Unexpected Refactoring around .Net C# Locking Issue

I’m constantly amazed by the insightfulness of ReSharper’s suggested refactorings (ReSharper is a Visual Studio Addin from JetBrains I use with C#). Today, I’ve been working on a threading problem where I’m getting crashes based on what seems like not proper locking across threads (they usually show up as some type of ugly update object or enum error).

My code starts like this:

public static List<DbProgressReport> DbProgressReportProperty { get; set; }

Then, I try wrapping the updates with a lock as follows (removing the set because I do it someplace else now)

public static List<DbProgressReport> DbProgressReportProperty
{
    get
    {
        lock (lockDbProgressReportProperty)
        {
            return _dbProgressReportList;
        }
    }
}

I then realize that I need to copy the data because it may change while the caller prints it so I decide to return a temporary copy of the data as follows:

public static List<DbProgressReport> DbProgressReportProperty
{
    get
    {
        lock (lockDbProgressReportProperty)
        {
            var dbProgressReportsNew = new List<DbProgressReport>();
            if (_dbProgressReportList != null)
            {
                // make temp copy to avoid locking issues on read
                foreach (var rec in _dbProgressReportList)
                {
                    dbProgressReportsNew.Add(rec);
                }
            }
            return dbProgressReportsNew;
        }
    }
}

I then notice that ReSharper has a suggestion.

Taking the suggestion, Resharper changes the code to:

lock (lockDbProgressReportProperty)
{
    // make temp copy to avoid locking issues on 
    return _dbProgressReportList.ToList();
}

I expected a bunch of AddRange type stuff, but ReSharper figured out that ToList() would do everything I needed!

Very cool.

Hope this helps.