Multi-threaded utility that logs some audit trails I'm trying to write a multi-threaded utility that logs some audit trails to the DB every 30 minutes, or whenever my storage data structure (a list in this case) exceeds a certain limit.

The code works well, but having said that, my manager said there could potentially be some issues in the method LogUserUsageData(...) where one thread tries to add a UserUsage object while another thread is in the lock trying to initialize the same list. 


Should I be declaring my list as volatile?
Should I be thinking about double-locking techniques?


Can anybody throw more light on this?

namespace Utilities
{
    public class UserUsageStatistics
    {
        private SynchronizedCollection&lt;UserUsage&gt; _statisticsList;
        private readonly Thread _persistenceThread;
        private readonly string _flag = String.Empty;

        public UserUsageStatistics()
        {
            _statisticsList = new SynchronizedCollection&lt;UserUsage&gt;();
            _persistenceThread = new Thread(ThreadWork);
            _persistenceThread.Start();
        }

        public void LogUserUsageData(UserUsage userUsage)
        {
            if (_statisticsList.Count &gt;= 100) PersistToDataStore();
            _statisticsList.Add(userUsage);
        }

        void ThreadWork()
        {
            while (true)
            {
                PersistToDataStore();
                Thread.Sleep(1800000);
            }
        }

        /// &lt;summary&gt;
        /// Persist the data to the data store.
        /// &lt;/summary&gt;
        void PersistToDataStore()
        {
            if (_statisticsList.Count == 0) return;
            try
            {
                SynchronizedCollection&lt;UserUsage&gt; tempList;
                lock (_flag)
                {
                    if (_statisticsList.Count == 0) return;

                    tempList = _statisticsList;
                    _statisticsList = new SynchronizedCollection&lt;UserUsage&gt;();
                }
                //TODO: Save to database
            }
            catch (Exception ex)
            {
            }
        }

    }
}
DO NOT LOCK ON EMPTY STRING

In .NET every string is stored only once in the AppDomain so if you are locking on empty string everything will stop until you release the lock.

The correct syncroot can be:

private readonly object _flag = new object();


And yes you should use double check locking. The other thing is thati would use a ConcurrentQueue instead of SynchronizedCollection.