CODEWITHSUNDEEP
c#multithreading.net-3.5
Dim NY
Dim NY

Reputation: 133

c# while loop runs extra time

Trying to make program run on list of urls and open limited number of threads to process them. Problem is that the while loop will execute when listPosition is 18, even that the siteUrls.Count is 18. But it will never show any of those test messageboxes i added. Thread code itself has a locker that does currentThreads-- in the end of thread's work.

Why does it start the while loop when the condition becomes (18<18)? Why does it try to launch thread with the 18 but it wont show a message box? Problem exists only when i put more threads than list items count. Its strange to me because i think it should not run the loop when listPosition equals _siteUrls.Count. I tried adding various "waiting" for threads to start and what not, but it still doesn't work.

p.s. when i add 5ms wait before increasing listPosition it becomes more stable. What exactly does it need to wait for so i can program it in more nicely?

p.p.s Thread doesn't operate any of the counters except doing lock (ThreadsMinus) { currentThreads--;}

  listPosition = 0;

    while (listPosition < siteUrls.Count)
    {
        if (listPosition == 18)
        {
            MessageBox.Show("18");
        }
        currentThreads++;
        var t = new Thread(() => Register(siteUrls[listPosition], listPosition));
        t.Start();

        if (listPosition == 18)
        {
            MessageBox.Show("18");
        }
        while (currentThreads >= maxThreads)
        {
            Thread.Sleep(50);
        }
         listPosition++;
    }

Upvotes: 1

Views: 1141

Answers (2)

Mike Parkhill
Mike Parkhill

Reputation: 5551

Try the following (note you need to lock currentTheads in here too):

while (listPosition < siteUrls.Count)
{


    lock(ThreadsMinus){
      // only start a new thread if we haven't hit the maximum yet, otherwise keep spinning
      if (currentThreads < maxThreads){
         currentThreads++;
         var t = new Thread(() => Register(siteUrls[listPosition], listPosition));
          t.Start();
          listPosition++;
      } 
    }

}

It will only spark off a new thread if you're under the maxThreads, but because not every iteration will start a thread you could iterate multiple times for any given listPosition until a thread becomes available. So even though you only have 18 urls in your list you could iterate over that while loop a thousand times before they've all been serviced depending on how long your Register method takes.

Upvotes: 1

Alexei Levenkov
Alexei Levenkov

Reputation: 100547

Your code manipulates counters (listPosition and currentThreads) in unsafe way. Please use Interlocked.Increment and Interlocked.Decrement or appropriate lock around access to each counter.

Also it is unclear from the sample where currentThreads is decreased (as pointed out by Mike Parkhill), which is the most likely reason of infinite loop.

Upvotes: 2

Related Questions