HypeZ
HypeZ

Reputation: 4127

Correct way to use multiple threads in C#

i'm doing a little download manager with C# and WPF.

In my logic there is a main thread (for UI), a thread for manage connection and a thread for each download, until a maximum download number i defined with a semaphore.

Every time the user want to download something it adds a custom object DownloadDetails in a shared queue, the connection thread dequeue it and make another thread do the actual download.

For a single file it works fine, but for multiple files looks like more threads try to download the same file, trying to write the same file on local drive and crashing when writing stream.

Here some snippets of my implementation

in main form:

private Thread connectionManager;

when clickin download i add the new download (they are all correct, i checked it) and start the connection thread only if its not already started:

downloadList.Enqueue(newDownload);
if (connectionManager == null || !connectionManager.IsAlive)
{
   connectionManager = new Thread((ThreadStart)delegate
   {
      ManageDownload();
   });
   connectionManager.SetApartmentState(ApartmentState.STA);
   connectionManager.IsBackground = true;
   connectionManager.Start();
}

the download manager thread do this:

private void ThreadStartingPoint()
{
    downloadDetails singleDownload = new downloadDetails();
    while (downloadList.TryDequeue(out singleDownload))
    {
        downloadSemaphore.WaitOne();
        Thread singleDownloadThread = new Thread((ThreadStart)delegate
        {
            myDownloadClass.Download(singleDownload);
        });
        singleDownloadThread.SetApartmentState(ApartmentState.STA);
        singleDownloadThread.IsBackground = false;
        singleDownloadThread.Start();
    }
}

after completing the download (or canceling, or getting an error), the single thread do a downloadSemaphore.Release();

the problems is that the myDownloadClass.Download looks to get multiple time the same parameter when starting 2 or more downloads quickly

i have to use delegate because single download threads needs to update UI on main thread

how can i solve this? Thank you :)

Upvotes: 1

Views: 310

Answers (1)

Henk Holterman
Henk Holterman

Reputation: 273179

This fragment

while (downloadList.TryDequeue(out singleDownload))
{
    downloadSemaphore.WaitOne();
    Thread singleDownloadThread = new Thread((ThreadStart)delegate
    {
        myDownloadClass.Download(singleDownload);
    });

    ...
}

is capturing the loop-variable.

The fix:

while (downloadList.TryDequeue(out singleDownload))
{
    string localCopy = singleDownload;
    downloadSemaphore.WaitOne();
    Thread singleDownloadThread = new Thread((ThreadStart)delegate
    {
        myDownloadClass.Download(localCopy);
    });

    ...
}

As a more general advice, i would use Tasks and maybe the Parallel class to build a downloadmanager.

Upvotes: 3

Related Questions