QueueUserWorkItem with WaitCallback getting the return value

Question

I'm running a site that imports product feeds from a variety of shops. These feeds can be quite huge, some are up to 1GB. Currently I import these by calling the import function in a loop:

For i As Integer = 0 To dtAllFeeds.Rows.Count - 1

    iImported = ImportFeed(dtAllFeeds(i).id)
    totalProductsImported += iImported
    lblStatus.Text += "FeedId: " + dtAllFeeds(i).id.ToString + "Items Imported: " + iImported.ToString

    If iImported = 0 Then
        MailFunctions.NotifyAdmin("feed error: dtAllFeeds(i).id.ToString)
    End If
Next i

lblStatus.Text += "Total imported: " + totalProductsImported.ToString

This works, but as the size or number of feeds increase, so does the time to process them. So I not so elegantly just increased the timeout:

Protected Sub Page_Init(ByVal sender As Object, ByVal e As System.EventArgs) Handles Me.Init
    _timeOut = Server.ScriptTimeout
    Server.ScriptTimeout = 36000 '10 hours
End Sub

Now, I want to run these tasks without having to wait for each task to complete before starting the next, so I tried the setup as described here, first with a test function TestMultiThread:

Protected Function TestMultiThread(ByVal Id As Integer, ByVal s As String) As Integer
    LogError("s = " + s)
    For i As Integer = 0 To (Id * 10000)
    Next i
    LogError(Id.ToString + " completed")
    Return Id * 10000
End Function

Protected Sub Page_Load(ByVal sender As Object, ByVal e As System.EventArgs) Handles Me.Load
Dim numThreads = 20
Dim toProcess = numThreads
Dim resetEvent = New ManualResetEvent(False)
Dim i As Integer
For i = 1 To numThreads
    ThreadPool.QueueUserWorkItem(New WaitCallback(Sub(state As Object)
                                                      TestMultiThread(i, toProcess.ToString)
                                                      If Interlocked.Decrement(toProcess) = 0 Then
                                                          resetEvent.[Set]()
                                                      End If
                                                  End Sub), Nothing)
Next i

resetEvent.WaitOne()
End Sub

I then get these errors logged:

s = 20
s = 20
s = 20
21 completed
21 completed
21 completed
s = 19
s = 18
s = 17
21 completed
21 completed
s = 16
21 completed
s = 15
21 completed
s = 14
21 completed
s = 13
21 completed
s = 12
21 completed
s = 11
21 completed
s = 10
21 completed
s = 9
21 completed
s = 8
21 completed
s = 7
21 completed
s = 6
21 completed
s = 5
21 completed
s = 4
21 completed
s = 3
21 completed
21 completed

I don't understand this order of logging, howcome I don't see s = <value> for 20 unique values (but s=20 even 3 times in a row in the beginning and missing s=2 and s=1? And why is i always 21 in function TestMultiThread?

Answer 1

You have several problems. First of all, I can't even compile it with For i = 0 To (i * 10000) in TestMultiThread, because you are also using i as the parameter name.

Second of all, the strangeness you're seeing is because you're passing the loop iterator i to TestMultiThread, which is a modified closure--you're capturing the variable itself rather than its value. By the time each thread pool delegate runs, the value of i is 21, having been incremented after each iteration of the loop body. To solve this problem, copy i to a local variable within the loop body and instead pass that local variable to TestMultiThread.

Finally, since this is being done in the context of ASP.NET, be aware that spinning up a bunch of new threads will rob the ASP.NET thread pool of threads it can use to handle incoming requests. Stephen Cleary explains:

• The request starts processing on an ASP.NET thread.
• Task.Run starts a task on the thread pool to do the calculations. The ASP.NET thread
  pool has to deal with (unexpectedly) losing one of its threads for
  the duration of this request.
• The original request thread is returned to the ASP.NET thread pool.
• When the calculation is complete, that
  thread completes the request and is returned to the ASP.NET thread
  pool. The ASP.NET thread pool has to deal with (unexpectedly) getting another thread.

ThreadPool.QueueUserWorkItem in your case is analagous to Task.Run in his example--it creates a background thread. If you want to do fire-and-forget in ASP.NET, look into using HostingEnvironment.QueueBackgroundWorkItem, as Cleary suggests. If you actually need to do anything with the feeds after you've imported them, then consider taking advantage of asynchronous programming to start each import before awaiting them all concurrently (I assume you are calling APIs--a naturally asynchronous operation--since you are importing the feeds "from a variety of shops").