Getting data async

Question

I need to handle concurrent requests by waiting for the result of an already running operation. Is this code safe or does it contain bugs?

private readonly object _gettingDataTaskLock = new object();
private Task<Data> _gettingDataTask;

public virtual Data GetData()
{
    Task<Data> inProgressDataTask = null;

    lock (_gettingDataTaskLock)
    {
        if (_gettingDataTask == null)
        {
            _gettingDataTask = Task.Factory.StartNew(() => GetDataInternal());
            _gettingDataTask.ContinueWith((task) =>
            {
                lock (_gettingDataTaskLock)
                {
                    _gettingDataTask = null;
                }
             },
             TaskContinuationOptions.ExecuteSynchronously);
         }

         inProgressDataTask = _gettingDataTask;
     }  

     try
     {
         return inProgressDataTask.Result;
     }
     catch (AggregateException ex)
     {
         _logger.ErrorException(ex, "An error occurs during getting full data");
         throw ex.InnerException;
     }
}

Answer 1

It is safe, but can be refactored using .NET built-in Lazy<T> class.

There can be several solutions depending on whether results of the GetDataInternal execution can be cached.

Simplest solution is when you can cache the results of the GetDataInternal execution for the lifetime of your parent object. In this case the code can be as simple as (move _lazyData initialization to constructor if GetDataInternal is not static):

private readonly Lazy<Data> _lazyData = new Lazy<Data>(GetDataInternal);

public virtual Data GetData()
{
    return _lazyData.Value; //skipped the logging of the error here as it doesn't add value
}

If you cannot use the cached results (your current logic), then the implementation can look like this:

private Lazy<Data> _lazyData = new Lazy<Data>(GetDataInternal);

public virtual Data GetData()
{
    var lazyData = _lazyData;

    try
    {
        return lazyData.Value;
    }
    finally
    {
        //one of the threads waiting for result will replace the _lazyData with new lazy instance as soon as the Data is calculated or exception is thrown
        Interlocked.CompareExchange(ref _lazyData, new Lazy<Data>(GetDataInternal), lazyData);
    }
}