AggregateException and Flatten

I have the following method that has been shortened for the sake of this review. The actual method has many more tasks. Essentially what I am trying to do is to create many tasks, fire them off, wait until they are all finished, and handle any exceptions generated. Does this look correct?

public static void RefreshCoaterCaches()
{
    try
    {
        CicApplication.CoaterDataLock.EnterWriteLock();

        List alarmTasks = new List();

        alarmTasks.Add(Task.Factory.StartNew(() =>
        {
            using (DistributorBackpressure44Logic logic = new DistributorBackpressure44Logic())
                logic.RefreshDistributorBackpressure44Cache();
        }));

        alarmTasks.Add(Task.Factory.StartNew(() =>
        {
            using (DeltaPressureLogic logic = new DeltaPressureLogic())
                logic.RefreshDeltaPressureCache();
        }));

        Task.WaitAll(alarmTasks.ToArray());
    }
    catch (System.AggregateException exception)
    {
        CicServerLogic.HandleUnexpectedAggregateException(exception.Flatten(), string.Empty);
    }
    catch (System.Exception exception)
    {
        CicServerLogic.HandleUnexpectedException(exception);
    }
    finally
    {
        CicApplication.CoaterDataLock.ExitWriteLock();
    }
}

I then have this method to handle any AggregateException thrown:

internal static void HandleUnexpectedAggregateException(AggregateException aggregateException)
{
    try
    {
        foreach (Exception exception in aggregateException.InnerExceptions) 
        {
         ...
         ...
         Do some stuff with the current exception, like logging etc.
         ...
        }
    }
    catch (Exception)
    {
    }
}

I suggested to think about following:

• Extract every step into the separate asynchronous method

• Think about more careful locking technique. Asynchronous operations by its nature could be long running and using global write lock can degrade performance dramatically. Maybe its possible acquire locks only during processing results.

• Global exception handling: in general global exception handling is a code smell. Its really hard to create generic logic for different operations. I suggested to use such techniques with some tools like MEF, but avoid using its manually.

Here slightly modified version of your code:

private static Task GetDistributorBackpressure44LogicAsync()
{
  Task.Factory.StartNew( () =>
  {
    using (DistributorBackpressure44Logic logic = new DistributorBackpressure44Logic())
                logic.RefreshDistributorBackpressure44Cache();
  }
}

private static Task DeltaPressureLogicAsync()
{
  Task.Factory.StartNew(() =>
  {
    using (DeltaPressureLogic logic = new DeltaPressureLogic())
                logic.RefreshDeltaPressureCache();
  }
}

public static void RefreshCoaterCaches()
{
    try
    {
        CicApplication.CoaterDataLock.EnterWriteLock();

        var t1 = GetDistributorBackpressure44LogicAsync();
        var t2 = DeltaPressureLogicAsync();

        // You can switch back to you list-based approach if you'll have
        // much more other operations
        Task.WaitAll(t1, t2);
    }
    catch (System.AggregateException exception)
    {
        // I'm really not sure that its a good idea to create "generic"
        // exception handling scheme. Maybe you should handle them manually right here.
        // And what is string.Empty is all about?
        CicServerLogic.HandleUnexpectedAggregateException(exception.Flatten(), string.Empty);
    }
    // You should not catch System.Exception thats why I removed it
    finally
    {
        CicApplication.CoaterDataLock.ExitWriteLock();
    }
}

Now, about your generic exception handler (I mentioned already that this technique can lead to errors hiding and complicated "generic" logic) (see my comments right in the code):

internal static void HandleUnexpectedAggregateException(AggregateException aggregateException)
{
    // What are we afraiding of, why we're using try/catch here?
    // We should now swallow unknown exception because it can lead to
    // hiding bugs in our code!
    try
    {
        // This technique is ok
        foreach (Exception exception in aggregateException.InnerExceptions) 
        {
         ...
         ...
         Do some stuff with the current exception, like logging etc.
         ...
        }
    }
    catch (Exception)
    {
       // I strongly not recommended to use empty catch blocks!!
    }
}

StackExchange Code Review Q#19028, answer score: 5