2

I'm attempting to make a multi-threaded download manager that has a limit of 4 concurrent downloads. In my research, I came across the following: C# Downloader: should I use Threads, BackgroundWorker or ThreadPool?

[edit] updated code:

Imports System.Net
Imports System.Collections.Concurrent
Imports System.ComponentModel
Imports System.Threading

Public Class Form1

    Const MaxClients As Integer = 4
    ' create a queue that allows the max items
    Dim ClientQueue As New BlockingCollection(Of WebClient)(MaxClients)

    ' queue of urls to be downloaded (unbounded)
    Dim UrlQueue As New Queue(Of String)()

    Dim downloadThread As Thread

    'Private Sub Form1_Load(sender As Object, e As EventArgs) Handles MyBase.Load
    Private Sub Button1_Click(sender As Object, e As EventArgs) Handles Button1.Click

        ' create four WebClient instances and put them into the queue
        For i As Integer = 0 To MaxClients - 1
            Dim cli = New WebClient()
            AddHandler cli.DownloadFileCompleted, AddressOf DownloadFileCompleted
            AddHandler cli.DownloadProgressChanged, AddressOf DownloadProgressChanged

            ClientQueue.Add(cli)
        Next

        ' Fill the UrlQueue here
        UrlQueue.Enqueue("http://www.gnu.org/licenses/gpl-1.0.txt")
        UrlQueue.Enqueue("http://www.gnu.org/licenses/gpl-2.0.txt")
        UrlQueue.Enqueue("http://www.gnu.org/licenses/gpl-3.0.txt")
        UrlQueue.Enqueue("http://www.gnu.org/licenses/lgpl-2.1.txt")
        UrlQueue.Enqueue("http://www.gnu.org/licenses/lgpl-3.0.txt")
        UrlQueue.Enqueue("http://www.gnu.org/licenses/fdl-1.1.txt")
        UrlQueue.Enqueue("http://www.gnu.org/licenses/fdl-1.2.txt")
        UrlQueue.Enqueue("http://www.gnu.org/licenses/fdl-1.3.txt")

        downloadThread = New Thread(AddressOf downloadQueue)
        downloadThread.IsBackground = True
        downloadThread.Start()
    End Sub

    Private Sub downloadQueue()
        ' Now go until the UrlQueue is empty
        While UrlQueue.Count > 0
            Dim cli As WebClient = ClientQueue.Take() ' blocks if there is no client available

            Dim url As String = UrlQueue.Dequeue()

            Dim fname As String = CreateOutputFilename(url)
            cli.DownloadFileAsync(New Uri(url), fname, New DownloadArgs(url, fname, cli))
            AppendText(url & " started" & vbCrLf)
        End While
    End Sub

    Private Sub DownloadProgressChanged(sender As Object, e As DownloadProgressChangedEventArgs)
        Dim args As DownloadArgs = DirectCast(e.UserState, DownloadArgs)
        ' Do status updates for this download
    End Sub

    Private Sub DownloadFileCompleted(sender As Object, e As AsyncCompletedEventArgs)
        Dim args As DownloadArgs = DirectCast(e.UserState, DownloadArgs)
        ' do whatever UI updates

        Dim url As String = "Filename" '<============I'd like to be able to pass the filename or URL but can't figure this out
        AppendText(url & " completed" & vbCrLf)

        ' now put this client back into the queue
        ClientQueue.Add(args.Client)
    End Sub

    Public Function CreateOutputFilename(ByVal url As String) As String
        Try
            Return url.Substring(url.LastIndexOf("/") + 1)
        Catch ex As Exception
            Return url
        End Try        
    End Function

    Private Delegate Sub SetTextCallback(text As String)

    Private Sub AppendText(text As String)
        If Me.TextBox1.InvokeRequired Then
            TextBox1.Invoke(New Action(Of String)(AddressOf AppendText), text)
            Return
        End If
        Me.TextBox1.AppendText(text)
        Me.TextBox1.SelectionStart = TextBox1.TextLength
        Me.TextBox1.ScrollToCaret()
    End Sub
End Class

Class DownloadArgs
    Public ReadOnly Url As String
    Public ReadOnly Filename As String
    Public ReadOnly Client As WebClient
    Public Sub New(u As String, f As String, c As WebClient)
        Url = u
        Filename = f
        Client = c
    End Sub
End Class

This will successfully download the first 4 files in the UrlQueue, but it then seems to freeze and no further files download. I'd imagine the problem lies in something minor I missed in the process of converting from C# to vb.net, but I can't seem to figure this out.

Community
  • 1
  • 1
Seymour
  • 23
  • 4

2 Answers2

0

You are blocking the ability for your async queue to process. Not sure this is the "Correct" way to do this but the changes here make it work:

While UrlQueue.Count > 0
    Do While ClientQueue.Count = 0
        Application.DoEvents()
    Loop
    Dim cli As WebClient = ClientQueue.Take() ' blocks if there is no client available
    Dim url As String = UrlQueue.Dequeue()
    Dim fname As String = CreateOutputFilename(url)  ' or however you get the output file name
    cli.DownloadFileAsync(New Uri(url), fname, New DownloadArgs(url, fname, cli))
End While
Steve
  • 5,585
  • 2
  • 18
  • 32
0

ClientQueue.Take() blocks the UI thread. Also, WebClient will want to raise the DownloadFileCompleted event on the UI thread - but it is already blocked by ClientQueue.Take(). You have a deadlock.

To resolve this, you got to move your blocking loop to another background thread.

YK1
  • 7,327
  • 1
  • 21
  • 28
  • I've modified the original code and this seems to work. I'm trying to figure out a way to notify when a given download completes, starting with something simple like `TextBox1.AppendText("Download of: " url & " completed" & vbCrLf)` but I can't seem to to get this to work - any ideas? – Seymour Oct 15 '13 at 21:56
  • `but I can't seem to to get this to work` - do you get any exception? – YK1 Oct 16 '13 at 02:35
  • System.InvalidOperationException - Cross-thread operation not valid: Control 'TextBox1' accessed from a thread other than the thread it was created on. Additionally, using url results in: 'url' is not declared. It may be inaccessible due to its protection level. I understand why I can't use url, but I'm not sure what to use instead. Verifying that individual downloads complete in the DownloadFileCompleted sub makes sense to me, but I'm not sure if it should be done elsewhere with these restrictions. – Seymour Oct 16 '13 at 14:43
  • I fixed the Cross-thread operation not valid issue by creating the AppendText Sub (I edited the original code to show this). My issue now is finding a way to either pass the filename or url to the DownloadFileCompleted sub so I can update the GUI to reflect when each individual download completes. – Seymour Oct 16 '13 at 16:47
  • 1
    don't you get the url from `args.Url` ? – YK1 Oct 16 '13 at 17:00
  • args.URL worked perfectly, I'm not sure how I missed that. One more minor issue: the Button1_Click sub works fine the first time, but when it's clicked a second time, it'll reach the `For i As Integer = 0 To MaxClients - 1` loop, but gets stuck there, which locks up the GUI and you have to forcefully close the app. I wanted to figure out how to fix this, mainly because I'll be changing from the set of links to a series of checkboxes, so the Button1_Click sub may be called more than once. – Seymour Oct 16 '13 at 20:04
  • That is because `ClientQueue` is already full with 4 instances. So, `ClientQueue.Add(cli)` will block. You should fill the `ClientQueue` only once in `Form_Load` event or something and not on every button click. – YK1 Oct 16 '13 at 20:24
  • Thanks, that worked perfectly. Through testing I'm now noticing that the number of simultaneous downloads increases with each Button1_Click. This seems to happen because of the `Dim cli = New WebClient()` in the for loop. I've tried different variations but haven't found a working solution. For example, if I set MaxClients = 2, on the first run it downloads 2 at a time, on the second run it downloads 4 at a time, and on the third run it downloads 6 at a time, etc. – Seymour Oct 19 '13 at 00:12
  • I ended up doing this before the for/next loop: `While ClientQueue.Count > 0` `ClientQueue.TryTake(cli)` `End While` – Seymour Oct 22 '13 at 17:38