Recurrent action on concurrent collection I have a bunch of concurrent threads carrying out operations which return alerts. The alerts have to be persisted to a DB, but this cannot be done concurrently as it may cause duplicate alerts being created.
To avoid duplicate alerts I have written a class containing a ConcurrentBag&lt;T&gt; with a Timer that triggers Action&lt;T&gt; (in this case, sending my pending alerts to the DB) at a set interval.

I'm wondering if what I wrote it is written in a way that will not cause problems in the execution in the future. If alerts get lost due to an unexpected exception that would be problematic. I'd appreciate comments/criticisms/suggestions for improvement.

Here's the code:

class PeriodicConcurrentBag&lt;T&gt;
{
    public int Count
    {
        get { return this.items.Count(); }
    }

    public bool HasItems
    {
        get { return this.items.Any(); }
    }

    public bool IsStopped { get; private set; }

    private int timerPeriod;
    private Timer timer;
    private ConcurrentBag&lt;T&gt; items;
    private Action&lt;IEnumerable&lt;T&gt;&gt; actionOnItems;

    public PeriodicConcurrentBag(Action&lt;IEnumerable&lt;T&gt;&gt; actionOnItems, int periodInSeconds)
    {
        this.timerPeriod = periodInSeconds * 1000;
        this.actionOnItems = actionOnItems;

        this.items = new ConcurrentBag&lt;T&gt;();
    }

    public void Start()
    {
        this.IsStopped = false;
        this.timer = new Timer(this.InternalPeriodicAction, null, this.timerPeriod, Timeout.Infinite);
    }

    public void Stop()
    {
        this.IsStopped = true;
    }

    public void AddItem(T item)
    {
        this.items.Add(item);
    }

    public void AddItems(IEnumerable&lt;T&gt; items)
    {
        this.items.AddRange(items);
    }

    private void InternalPeriodicAction(object state)
    {
        if (!this.IsStopped)
        {
            var currentItems = Interlocked.Exchange(ref this.items, new ConcurrentBag&lt;T&gt;());
            this.actionOnItems(currentItems);

            if (!this.IsStopped)
                this.Start();
        }
    }
}


EDIT: The AddRange() method is an extension method I created for ConcurrentBag.
Your Stop method does not actually stop the timer. This is 


really counter-intuitive
wastes CPU




There is also a racing condition. If someone calls Stop in-between those two lines:

           if (!this.IsStopped)
            this.Start();


the Stop call will be ignored. The moral of the story: just take the lock.



Timer implements IDisposable. So you should call Dispose when you no longer need it (in Stop method, for example.) 



I would refactor actionOnItems into a public event.



int for time period is ambiguous and limiting. I can not use your class, if I want to send updates every 0.5 seconds, and I can easily make an error by assuming, that int represent milliseconds. You can eliminate those problems by replacing it with TimeSpan.



Edit: Also your AddRange method rises some red flags. I assume it is not an atomic operation, and you add items to the bag one by one. What will happen, if AddRange is called right before Interlocked.Exchange(ref this.items, new ConcurrentBag&lt;T&gt;()); ? Do you think the new items will go to the new bag or to the old one? How is your extension method synchronized with the rest of your code? You are walking on the really thin ice here. If you have a limited experience with multi-threading, I suggest you start simple. Remove ConcurrentBag, use regular list, but lock every single access point (both  public AND private, methods AND properties). Once this is working, test if it meats your performance requirements. If it does, leave it at that. There is no point in fighting over nanoseconds if


You do not actually need those nanoseconds


and


You get an extremely bug-prone code as a result