Null reference - Task ContinueWith()

Question

For the following piece of code (.NET v4.0.30319) I am getting a null reference exception indicated below in the second continuation.

Most interestingly this issue has only occurred in machines with 8GB RAM but other users have 16GB and more and they haven't reported any issue, and it is a very intermittent issue which leads me to suspect a garbage collection issue.

The GetData() can be called multiple times so the first continuation of _businessObjectTask will only be called once as _businessObjects will have been populated from that point forward.

I imagine the Object reference not set to an instance of an object exception is being thrown because either

1. _businessObjectTask is null and it can't continue from a null task.
2. items variable which is passed in as a parameter is null somehow

The line number in my log file (748) points to the one highlighted below and not the lambda expression which points to #1 above instead of #2. I've played around in Visual Studio and each of the lines following businessObjectTask.ContinueWith() are considered different i.e. if it was a null reference within the lambda expression it would give a different line number to 748

Any help would be greatly appreciated.

Edit: This isn't related to What is a NullReferenceException, and how do I fix it? because that is a much more basic explanation of null reference whereas this is much more complicated and subtle.

Exception

Full details of the stack trace (edited for simplicity with dummy class and namespace names)

Object reference not set to an instance of an object.
   at ApplicationNamespace.ClassName`1.<>c__DisplayClass4e.<GetData>b__44(Task`1 t) in e:\ClassName.cs:line 748
   at System.Threading.Tasks.ContinuationTaskFromResultTask`1.InnerInvoke()
   at System.Threading.Tasks.Task.Execute()

Code

private static IDictionary<string, IBusinessObject> _businessObjects;
private Task<IDictionary<string, IBusinessObject>> _businessObjectTask;

public Task GetData(IList<TBusinessItem> items))
{
    Log.Info("Starting GetData()");

    if (_businessObjects == null)
    {
        var businessObjectService = ServiceLocator.Current.GetInstance<IBusinessObjectService>();

        _businessObjectTask = businessObjectService.GetData(CancellationToken.None)
        .ContinueWith
        (
            t => 
            {
                _businessObjects = t.Result.ToDictionary(e => e.ItemId);

                return _businessObjects;
            },
            CancellationToken.None,
            TaskContinuationOptions.OnlyOnRanToCompletion,
            TaskScheduler.Current
        );
    }


    var taskSetLEData = _businessObjectTask.ContinueWith // Line 748 in my code - "Object reference not set to an instance of an object." thrown here
    (
        task =>
        {
            items.ToList().ForEach
            (
                item =>
                {
                    IBusinessObject businessObject;

                    _businessObjects.TryGetValue(item.Id, out businessObject);
                    item.BusinessObject = businessObject;
                }
            );
        },
        CancellationToken.None,
        TaskContinuationOptions.OnlyOnRanToCompletion, 
        TaskScheduler.Default
    );
}

Resolution:

So having used what I've learned from this question I went back to the original code and figured it all out.

Turns out the reason for this NRE was because the _businessObjectTask is non-static where as the _businessObjects is static.

This means that _businessObjects is null the first time GetData() is called which then sets _businessObjectTask to non-null. Then when _businessObjectTask.ContinueWith gets called it is non-null and continues without problem.

However if a second instance of this class above is instantiated, _businessObjects has already been populated so _businessObjectTask remains null. Then when _businessObjectTask.ContinueWith gets called, a NRE on _businessObjectTask is thrown.

There were a couple of options I could have taken but I ended up removing the _businessObjectTask to a synchronous method call which means that I dont need to use the continuation anymore and I set _businessObjects once.

Answer 1

This is a synchronization problem.

You are assuming that _businessObjectTask is always assigned before _businessObjects.

That is, however, not guaranteed. It is possible that your continuation that assign _businessObjects is executed inline and therefore before businessObjectService.GetData(...).ContinueWith(...) returns.

// This assignment could happend AFTER the inner assignment.
_businessObjectTask = businessObjectService.GetData(CancellationToken.None)
    .ContinueWith
    (
        t => 
        {
           // This assignment could happen BEFORE the outer assignment.
            _businessObjects = t.Result.ToDictionary(e => e.ItemId);              

Therefore, it is possible that _businessObjects is not null although _businessObjectTask is null.

If a concurrent thread would enter your GetData method at that time it would clearly not enter

if (_businessObjects == null) // not entered because it's not null
{
    ...
}

...and instead continue with

var taskSetLEData = _businessObjectTask.ContinueWith // line 748

...which will cause a null reference exception since _businessObjectTask is null.

---

Here's how you could simplify your code and resolve this synchronization problem:

private Lazy<Task<IDictionary<string, IBusinessObject>>> _lazyTask =
    new Lazy<Task<IDictionary<string, IBusinessObject>>>(FetchBusinessObjects);

private static async Task<IDictionary<string, IBusinessObject>> FetchBusinessObjects()
{
    var businessObjectService = ServiceLocator.Current.GetInstance<IBusinessObjectService>();
    return await businessObjectService.GetData(CancellationToken.None).ToDictionary(e => e.ItemId);
}

public async Task GetData(IList<TBusinessItem> items)
{
    Log.Info("Starting GetData()");

    var businessObjects = await _lazyTask.Value;

    items.ToList().ForEach
    (
        item =>
        {
            IBusinessObject businessObject;
            businessObjects.TryGetValue(item.Id, out businessObject);
            item.BusinessObject = businessObject;
        }
    );
}

Notes:

• Using Lazy<T> to ensure that the business object service is only invoked once (per instance of this class, whatever it is).

• Using async/await to simplify code.

• You may want to consider declaring _lazyTask as static. In your code there seem to be a mixup between static/non-static fields. I cannot know which is right for you.