CODEWITHSUNDEEP
c#asp.netmultithreading
thekaveman
thekaveman

Reputation: 4399

Strange place for threading?

I'm maintaining some existing pages and came across a bit of code from System.Threading and I'm not sure what to make of it.

(this is a the gist of it)

protected void Page_Load(object sender, EventArgs e)
{
    XmlDocument xmlDoc = GetFromCMS();
    var nodes = xmlDoc.SelectNodes("/xpath/to/nodes");

    int i = 0;
    while (i < nodes.Count)
    {
        //do stuff with nodes[i]

        //last line in while loop - this is where I'm confused
        Math.Max(System.Threading.Interlocked.Increment(ref i), i - 1);
    }
}

Does it makes sense to do something like this? Couldn't i be incremented ala i++ instead? I'm not versed in multithreading, but given that there is no other threading code on this page and nothing really "special" happening (no extra threads being created, etc), it seems a little strange to me.

Thanks for your assistance!

Upvotes: 1

Views: 122

Answers (3)

Juliet
Juliet

Reputation: 81516

Your intuition is correct -- the code is a little strange, would probably make a good submission to The DailyWTF.

I'm not sure of the original developers motives, but without any other context, the method appears to be threadsafe. You should be able to increment i using i++; with no risk.

Even better, you can eliminate i by rewriting as a foreach instead:

protected void Page_Load(object sender, EventArgs e)
{
    XmlDocument xmlDoc = GetFromCMS();
    var nodes = xmlDoc.SelectNodes("/xpath/to/nodes");

    foreach(var node in nodes)
    {
        //do stuff with node
    }
}

Upvotes: 7

Mitch Wheat
Mitch Wheat

Reputation: 300559

i is a local variable (and not shared with any other threads), so a simple i++ is safe.

So, replace this:

Math.Max(System.Threading.Interlocked.Increment(ref i), i - 1);

with this

i++;

Or as a commenter pointed out, replace with a simple for or foreach loop!

Upvotes: 4

Alex Moore
Alex Moore

Reputation: 3455

Unless you are sharing the local i variable with other threads somehow (highly doubtful), i++ would work just as well, without the overhead of the interlocking increment function.

The only way I could see that happening is if you are passing i by reference within the body of the loop to another function, which would then share it. If it's just used locally you are fine.

Upvotes: 1

Related Questions