0

I'm very new to threading, so my implementation is no doubt very rudimentary.

I'm attempting to spawn 8 or more threads that each download a file using FTPWebRequest, to be run in parallel.

All the threads seem to be spawned and do their work, but only two ever do work at any one time. They seem to get queued.

Can anyone suggest what I may be doing wrong and how to solve the issue?

private void btnDownload_Click(object sender, EventArgs e)
    {
        int itemCount = lBoxList.Items.Count;
        string[] listOfFilesToDownload = new string[itemCount];

        for (int i = 0; i < itemCount; i++)
        {
            listOfFilesToDownload[i] = (string)lBoxList.Items[i];
        }

        FtpParallelDownload("ftp://ftp.somedomain.com/sub/sub2/sub3/", (int)this.nudNumberOfThreads.Value, listOfFilesToDownload, tBoxDownloadPath.Text);

    }

public static void FtpParallelDownload(string serverUri, int maxNumberOfThreads, string[] listOfFilesToDownload, string downloadPath)
    {
        int progressPercent = 0;

        // Validate number of threads requested
        if (!(maxNumberOfThreads >= 1))
        {
            ArgumentException e = new ArgumentException();
            throw e;
        }

        // Calc number of files based on array length
        int numberOfFiles = listOfFilesToDownload.Length;

        // Don't spawn more threads than files
        if (maxNumberOfThreads > numberOfFiles)
        {
            maxNumberOfThreads = numberOfFiles;
        }            

        // Thread spawning

        List<Thread> threadPool = new List<Thread>();

        int runningThreadCount = 0;



        for (int i = 0; i < numberOfFiles; i++)
        {
            if (runningThreadCount < maxNumberOfThreads)
            {
                Thread workerThread = new Thread(() => FtpDownloadFile(serverUri, listOfFilesToDownload[i], downloadPath));
                threadPool.Add(workerThread);
                workerThread.Start();
                runningThreadCount++;                   
            }
            else
            {
                i--;
            }

            List<Thread> removeThreadList = new List<Thread>();
            foreach (Thread t in threadPool)
            {
                if (!t.IsAlive)
                {
                    removeThreadList.Add(t);
                }
            }

            foreach (Thread t in removeThreadList)
            {
                threadPool.Remove(t);
                runningThreadCount--;
            }
            removeThreadList.Clear();
        }

        MessageBox.Show("DOWNLOADS COMPLETE!");
    }

    private static void FtpDownloadFile(string serverUri, string fileName, string downloadPath)
    {
        try
        {
            FtpWebRequest request = (FtpWebRequest)FtpWebRequest.Create(new Uri(serverUri + fileName));
            request.Method = WebRequestMethods.Ftp.DownloadFile;
            request.Credentials = new NetworkCredential("anonymous", "noreply@mydomain.com");
            FtpWebResponse response = (FtpWebResponse)request.GetResponse();
            Stream responseStream = response.GetResponseStream();
            FileStream writeStream = new FileStream(downloadPath + "/" + fileName, FileMode.Create);

            int Length = 4096;
            Byte[] buffer = new Byte[Length];
            int bytesRead = responseStream.Read(buffer, 0, Length);
            while (bytesRead > 0)
            {
                writeStream.Write(buffer, 0, bytesRead);
                bytesRead = responseStream.Read(buffer, 0, Length);
                Thread.Sleep(10);

            }
            writeStream.Close();
            response.Close();
            responseStream.Close();
        }

        catch (WebException wEx)
        {
            MessageBox.Show(wEx.Message, "Download Error");
        }
        catch (Exception ex)
        {
            MessageBox.Show(ex.Message, "Download Error");
        }
    }

threadPool is a Thread<List> that I am using to monitor the number of threads that are alive so as to spawn more when one is finished work.

Quantum_Kernel
  • 303
  • 1
  • 7
  • 19
  • I fear that you need to put in the implementation of the FtpDownLoadFile if that is a custom method as your problem can originate from the implementation there. – Thomas Nov 23 '15 at 07:11
  • 8
    _"threadPool is a Thread that I am using to monitor the number of threads that are alive so as to spawn more when one is finished work"_ - Due to the mindbogglingly complexity of creating a thread pool; scheduling and dynamic goodness, one should generally refrain from creating their own implementation. Someone such as yourself who admits to being _"new to threading"_ should definitely not craft their own pool mechanism. –  Nov 23 '15 at 07:18
  • I'm a very satisfied user of [SmartThreadPool](https://github.com/amibar/SmartThreadPool) since years. – Uwe Keim Nov 23 '15 at 07:26
  • 2
    Without more detail, including a [mcve], it would be impossible to say for sure what's going on. But I would say there's a very high chance you are simply seeing the default connection limit. See e.g. [HttpWebRequest takes a long time to send when there are a bunch at once from client](https://stackoverflow.com/q/3612992) and [C# Manual Threading](https://stackoverflow.com/q/1296134) for possible relevant discussion. Please improve the question if you still need help after reading those. – Peter Duniho Nov 23 '15 at 07:28
  • 1
    "I'm attempting to spawn 8 or more threads that each download an file by FTPWebRequest". What you are doing wrong is that you are trying to use multithreading. You should try using asynchronous I/O instead. – Aron Nov 23 '15 at 08:50
  • 1
    Nothing wrong with using threads here, 8 threads are nothing. This should basically work. Peter is right. – usr Nov 23 '15 at 10:33
  • I don't think it's the server connection limit as I can download 10 files from the site in question at one time using Filezilla. – Quantum_Kernel Nov 23 '15 at 10:52
  • 1
    Omg, `i--;` is such a creative way to create an infinite loop :) Your whole thread management should be replaced with PLINQ or `Parallel.ForEach`. Delete all of this. – usr Nov 23 '15 at 11:04
  • 1
    In fact you might post this to the Code Review site. They will turn up about a dozen issues which will be quite instructive. – usr Nov 23 '15 at 11:05
  • Just a note: make sure that whatever you ask about on [Code Review](http://codereview.stackexchange.com) is about **already working** code. – Kaz Nov 23 '15 at 11:07
  • @Quantum_Kernel the connection limit is imposed by the *client*. FileZilla probably changes the connection limit in code. In any case, .NET offers a *lot* of well tested ways to execute concurrent and asynchronous operations. There's no reason to reinvent them – Panagiotis Kanavos Nov 23 '15 at 11:28

1 Answers1

1

You really should use an existing library rather than rolling your own.

Here's how you could code this using Microsoft's Reactive Framework (NuGet "Rx-WinForms").

public static void FtpParallelDownload(
    string serverUri, int maxNumberOfThreads,
    string[] listOfFilesToDownload, string downloadPath)
{
    listOfFilesToDownload
        .ToObservable()
        .Select(x => Observable.Start(() => FtpDownloadFile(serverUri, x, downloadPath)))
        .Merge(maxNumberOfThreads)
        .ObserveOn(this)
        .ToArray()
        .Subscribe(xs => { }, () => { MessageBox.Show("DOWNLOADS COMPLETE!"); });
}

Done.

Enigmativity
  • 113,464
  • 11
  • 89
  • 172