0

I have a while loop that I want to init threads however the below code ends up creating almost 50 threads. Far above my intended limit of 5 Not sure what I'm doing wrong.

public static SemaphoreSlim scraperSemaphore = new SemaphoreSlim(5);

private void startScraper()
        {

            while (Core.Scraper.UsernameQueue.Count > 0)
            {

                string uName = Core.Scraper.UsernameQueue.Dequeue();                
                scraperSemaphore.Wait();
                Thread t = new Thread(() => Core.Scraper.scrapeFunction(uName));
                t.Start();
                scraperSemaphore.Release();

            }
                      

        }
  • 2
    You only need the lock in order to *start* a new thread. There's nothing which says the lock needs to be held until the thread finishes. So you can start 5 threads concurrently, but once you've finished starting one thread someone else can come along and start another, regardless of whether the thread you've started has finished yet. `scrapeFunction` needs to be the thing which releases the semaphore, when it's done processing (or otherwise exits) – canton7 Mar 03 '21 at 15:31
  • See also https://stackoverflow.com/questions/10806951/how-to-limit-the-amount-of-concurrent-async-i-o-operations for async/await-specific approaches. – Peter Duniho Mar 03 '21 at 17:59

1 Answers1

0

I would suggest using Parallel.Foreach together with a blocking collection instead.

var queue = new BlockingCollection<string>(new ConcurrentQueue<string>());
var options = new ParallelOptions(){MaxDegreeOfParallelism = 3}
Parallel.ForEach(queue.GetConsumingEnumerable() , options , (item) => {...})

You the combination of parallel.Foreach with Blockingcollection can create some unneeded synchronization that may or may not affect performance. There are some extensions that may solve that. See blog post ParallelExtensionsExtras Tour – #4 – BlockingCollectionExtensions by Stephen Toub .

JonasH
  • 15,400
  • 1
  • 8
  • 16