Not every result of IProgress comes out of the task

Question

Consider following implementation, a method accepts an IProgress<int>, iterates over 10000 objects. The numbers array variable returns 10000 objects, but the IProgress<int> reports only between 9970 - 9980 objects. It varies per run, so some get "lost".

    protected async override Task<int[]> CollectDataAsyncImpl(IProgress<int> progress) {                
        return await Task.Run<int[]>(() => {

            var numbers = new List<int>();

            foreach (var idx in new Int32Range(1, 10000).AsEnumerable().Index()) {                                            

                numbers.Add(idx.Value);                    

                if (progress != null) {
                    progress.Report(idx.Value);
                }

            }

            return numbers.ToArray();
        });
    }

As reference, here's the test I ran. It fails at the third assert Assert.Equal(10000, result[9999]);.

[Fact]
async void ReportsProgress() {            
    var sut = new IntegerCollector();
    var result = new List<int>();
    var output = await sut.CollectDataAsync(new Progress<int>(i => result.Add(i)));
    Assert.Equal(10000, output.Length);
    Assert.Equal(1, result[0]);
    Assert.Equal(10000, result[9999]);
}

Clearly I'm doing something wrong, or I don't understand the internals of task/threading. Is my implementation of IProgress<int> to new Progress<int>(i => result.Add(i)) not correct? Should I make that thread safe, and if so, how do I do that?

GitHub has the code which you can clone & test with if need be: https://github.com/KodeFoxx/Kf.DataCollection/tree/master/Source/Kf.DataCollection

It is a standard threading race bug, IProgress.Report() uses the SynchronizationContext.Post() method to invoke the delegate. Not synchronous like Send(), it completes later. Use IProgress only for non-time-critical progress reporting to the user interface. — Hans Passant, Oct 03 '15 at 01:49

score 3 · Accepted Answer · answered Oct 03 '15 at 01:40

3

It is probably because of how Progress<T> is implemented. When created, Progress<T> captures synchronization context and uses it to execute i => result.Add(i). Since you are running a test, I assume that there is no synchronization context. In this case Progress<T> uses default SynchronizationContext, which posts work items to thread pool (ThreadPool.QueueUserWorkItem). Your task completes before thread pool processes all queued items, and it perfectly explains results inconsistency.

A simple way to check if this is the case: change IProgress<int> argument to Action<int> and pass i => result.Add(i) delegate directly, without wrapping it with a Progress<T>.

answered Oct 03 '15 at 01:40

max

33,369
7
73
84

Thanks. Does that mean I would have to either roll my own IProgress implementation for my tests, or should I rely on the fact that Progress is tested and will work as advertised since it basically is a .Net element. And to just rely on it working, but then only test if I get at least one callback from the progress function? – Yves Schelpe Oct 03 '15 at 13:17
1

I wrapped the whole test code in a `Task.Run(() => { ... });` and then it did work. I assume it is because then it has a context now. See passing test code at http://pasteboard.co/16jbGC5X.png. I assume this would be a correct way to give it context, or is it overkill? – Yves Schelpe Oct 03 '15 at 13:47
1

1. Yes, you can provide a simple `IProgress` implementation for tests. .NET `Progress` implementation works as intended, but note that its purpose is to report progress to UI (that's why all the `SynchronizationContext` stuff is used). If you want to consume actual results in async fashion, consider `IObservable` & `IObserver` instead. – max Oct 03 '15 at 13:52

Not every result of IProgress comes out of the task

1 Answers1