Encountering unexpected Interlocked behaviour?

Question

I'm writing a web link checker program and encountering behaviour with Interlocked that I can't explain. First, here's an abridged version of the code:

public class LinkCheckProcessor
{
    private long _remainingLinks;

    public event EventHandler<LinksCheckedEventArgs> AllLinksChecked;

    private void ProcessLinks(List<Link> links)
    {
        foreach (Link link in links)
        {
            ProcessLink(link);
        }
    }

    private void ProcessLink(Link link)
    {
        var linkChecker = new LinkChecker(link);
        linkChecker.LinkChecked += LinkChecked;
        Interlocked.Increment(ref _remainingLinks);
#if DEBUG
        System.Diagnostics.Debug.WriteLine(String.Format("LinkChecker: Checking link '{0}', remaining: {1}", link, Interlocked.Read(ref _remainingLinks)));
#endif
        linkChecker.Check();
    }

    void LinkChecked(object sender, LinkCheckedEventArgs e)
    {
        var linkChecker = (LinkChecker)sender;

        Interlocked.Decrement(ref _remainingLinks);
#if DEBUG
        System.Diagnostics.Debug.WriteLine(String.Format("LinkChecker: Checked link '{0}', remaining: {1}", linkChecker.Link, Interlocked.Read(ref _remainingLinks)));
#endif
        if (Interlocked.Read(ref _remainingLinks) == 0)
        {
            OnAllLinksChecked(new LinksCheckedEventArgs(this.BatchId, this.Web.Url));
        }
    }
}

What I'm seeing in the debug output are things like:

• LinkChecker: Checked link 'http://serverfault.com', remaining: 1
• LinkChecker: Checked link 'http://superuser.com', remaining: 0
• LinkChecker: Checked link 'http://stackoverflow.com', remaining: -1

I don't understand why (in some code runs) _remainingLinks is becoming negative. This is also having the side effect of causing the AllLinksChecked event from being fired too early. (By the way, the code above contains the only places that _remainingLinks is touched.)

What am I doing wrong?

Answer 1

I'm going to go out on a limb and propose that LinkChecker is firing more than one event for a call to Check(). Short of this, I can't see how the value could possibly go negative.

Answer 2

Your AllLinksChecked logic is definitely wrong, since the counter could go 0->1, 1->0, 0->1, 1->0, 0->1, 1->0 and thus reach zero multiple times.

But I don't see how the count could ever go negative. Are these the only places that _remainingLinks appears in your code?

---

The first problem could be fixed just by removing the increment code from ProcessLink and having ProcessLinks initialize the count to links.Count before starting the loop:

Interlocked.Exchange(ref _remainingLinks, links.Count)`

---

The links argument isn't being written from other threads while ProcessLinks is running, is it?