1

I would like to use the QueueUserWorkItem from the ThreadPool. When I use the following code everything works well.

private int ThreadCountSemaphore = 0;
private void (...) {

var reportingDataList = new List<LBReportingData>();
ThreadCountSemaphore = reportingDataList.Count;
using (var autoResetEvent = new AutoResetEvent(false)) {
   ThreadPool.QueueUserWorkItem((o) => this.FillReportingData(settings, reportingDataList[0], autoResetEvent));
   ThreadPool.QueueUserWorkItem((o) => this.FillReportingData(settings, reportingDataList[1], autoResetEvent));
   ThreadPool.QueueUserWorkItem((o) => this.FillReportingData(settings, reportingDataList[2], autoResetEvent));
}
}

private void FillReportingData(...) {
if (Interlocked.Decrement(ref this.ThreadCountSemaphore) == 0) {
                waitHandle.Set();
                }
}

But when I use a list instead the single method calls, then my program crash without an exception.

private void (...) {

var reportingDataList = new List<LBReportingData>();
ThreadCountSemaphore = reportingDataList.Count;
using (var autoResetEvent = new AutoResetEvent(false)) {
   ThreadPool.QueueUserWorkItem((o) => this.FillReportingData(settings, reportingDataList[i], autoResetEvent));
}
}

What do i wrong? What should I change?

Update

Sorry, I've made a fault in the code. I use .NET 2.0 with VS2010. Here's the complete code:

private int ThreadCountSemaphore = 0;

        private IList<LBReportingData> LoadReportsForBatch() {
            var reportingDataList = new List<LBReportingData>();
            var settings = OnNeedEntitySettings();

            if (settings.Settings.ReportDefinition != null) {
                var definitionList = new List<ReportDefinitionen> { ReportDefinitionen.OrgStatus, ReportDefinitionen.Mittelwerte, ReportDefinitionen.Verteilungsstatistik };
                using (var autoResetEvent = new AutoResetEvent(false)) {
                    foreach (var reportDefinition in definitionList) {
                        foreach (DataRow row in settings.Settings.ReportDefinition.Select("AuswertungsTyp = " + (int)reportDefinition)) {
                            reportingDataList.Add(new LBReportingData { SourceData = row, ReportType = reportDefinition });
                        }
                    }

                    ThreadCountSemaphore = reportingDataList.Count;

                    foreach(var reportingDataItem in reportingDataList) {                                       
                        ThreadPool.QueueUserWorkItem((o) => this.FillReportingData(settings, reportingDataItem, autoResetEvent));
                    }
                    autoResetEvent.WaitOne();
                }
            }
            return reportingDataList;
        }

private void FillReportingData(IEntitySettings<DSLBUReportDefinition> settings, LBReportingData reportingData, AutoResetEvent waitHandle){

            DoSomeWork();
            if (Interlocked.Decrement(ref this.ThreadCountSemaphore) == 0) {
                waitHandle.Set();
            }
        }

Thanks

Andras Vass
  • 11,478
  • 1
  • 37
  • 49
Dani
  • 31
  • 5
  • 1
    Very unclear, where is the value of "i" supposed to come from? – Hans Passant Sep 15 '10 at 17:29
  • If this is C#, the ellipsis is not an allowed parameter. Therefore, in order to better troubleshoot this issue, we'll need to know the parameter types you've declared for `FillReportingData`. Also, in example 2, you're still only passing a single `LBReportingData` object, not the entire list. – Nathan Wheeler Sep 15 '10 at 17:31

2 Answers2

2

You are disposing the WaitHandle immediately after queueing the work items. There is race between the call to Dispose in the main thread and Set in the worker thread. There may be other problems, but it is difficult to guess because the code is incomplete.

Here is how the pattern is suppose to work.

using (var finished = new CountdownEvent(1))
{
  foreach (var item in reportingDataList)
  {  
     var captured = item;
     finished.AddCount(); 
     ThreadPool.QueueUserWorkItem(
       (state) =>
       {
         try
         {
           DoSomeWork(captured); // FillReportingData?
         }
         finally
         {
           finished.Signal();
         }
       }, null);
  } 
  finished.Signal();
  finished.Wait();
}

The code uses the CountdownEvent class. It is available in .NET 4.0 or as part of the Reactive Extensions download.

Brian Gideon
  • 47,849
  • 13
  • 107
  • 150
  • +1 The disposing of the AutoResetEvent was the first red flag I seen, however, without the full code it is difficult to pinpoint it exactly, but I would be adamant that this is part of the overall problem. – James Sep 15 '10 at 18:06
0

As Hans pointed out, it is not clear where "i" is coming from. But also I can see your disposing block going out and disposed because you are not using WaitOne on it (or you have not copied that part of code).

Also I would prefer to use WaitAll and not using interlocked.

Aliostad
  • 80,612
  • 21
  • 160
  • 208
  • `WaitAll` in this particular scenario would be fine, however, it does have it's limitations at higher volumes. – James Sep 15 '10 at 17:46