views:

101

answers:

3

With business logic encapsulated behind synchronous service calls e.g.:

interface IFooService
{
    Foo GetFooById(int id);
    int SaveFoo(Foo foo);
}

What is the best way to extend/use these service calls in an asynchronous fashion?

At present I've created a simple AsyncUtils class:

public static class AsyncUtils
{
    public static void Execute<T>(Func<T> asyncFunc)
    {
        Execute(asyncFunc, null, null);
    }

    public static void Execute<T>(Func<T> asyncFunc, Action<T> successCallback)
    {
        Execute(asyncFunc, successCallback, null);
    }

    public static void Execute<T>(Func<T> asyncFunc, Action<T> successCallback, Action<Exception> failureCallback)
    {
        ThreadPool.UnsafeQueueUserWorkItem(state => ExecuteAndHandleError(asyncFunc, successCallback, failureCallback), null);
    }

    private static void ExecuteAndHandleError<T>(Func<T> asyncFunc, Action<T> successCallback, Action<Exception> failureCallback)
    {
        try
        {
            T result = asyncFunc();
            if (successCallback != null)
            {
                successCallback(result);
            }
        }
        catch (Exception e)
        {
            if (failureCallback != null)
            {
                failureCallback(e);
            }
        }
    }
}

Which lets me call anything asynchronously:

AsyncUtils(
     () => _fooService.SaveFoo(foo),
     id => HandleFooSavedSuccessfully(id),
     ex => HandleFooSaveError(ex));

Whilst this works in simple use cases it quickly gets tricky if other processes need to coordinate about the results, for example if I need to save three objects asynchronously before the current thread can continue then I'd like a way to wait-on/join the worker threads.

Options I've thought of so far include:

  • having AsyncUtils return a WaitHandle
  • having AsyncUtils use an AsyncMethodCaller and return an IAsyncResult
  • rewriting the API to include Begin, End async calls

e.g. something resembling:

interface IFooService
{
    Foo GetFooById(int id);
    IAsyncResult BeginGetFooById(int id);
    Foo EndGetFooById(IAsyncResult result);
    int SaveFoo(Foo foo);
    IAsyncResult BeginSaveFoo(Foo foo);
    int EndSaveFoo(IAsyncResult result);
}

Are there other approaches I should consider? What are the benefits and potential pitfalls of each?

Ideally I'd like to keep the service layer simple/synchronous and provide some easy to use utility methods for calling them asynchronously. I'd be interested in hearing about solutions and ideas applicable to C# 3.5 and C# 4 (we haven't upgraded yet but will do in the relatively near future).

Looking forward to your ideas.

+3  A: 

Given your requirement to stay .NET 2.0 only, and not work on 3.5 or 4.0, this is probably the best option.

I do have three remarks on your current implementation.

  1. Is there a specific reason you're using ThreadPool.UnsafeQueueUserWorkItem? Unless there is a specific reason this is required, I would recommend using ThreadPool.QueueUserWorkItem instead, especially if you're in a large development team. The Unsafe version can potentially allow security flaws to appear as you lose the calling stack, and as a result, the ability to control permissions as closely.

  2. The current design of your exception handling, using the failureCallback, will swallow all exceptions, and provide no feedback, unless a callback is defined. It might be better to propogate the exception and let it bubble up if you're not going to handle it properly. Alternatively, you could push this back onto the calling thread in some fashion, though this would require using something more like IAsyncResult.

  3. You currently have no way to tell if an asynchronous call is completed. This would be the other advantage of using IAsyncResult in your design (though it does add some complexity to the implementation).


Once you upgrade to .NET 4, however, I would recommend just putting this in a Task or Task<T>, as it was designed to handle this very cleanly. Instead of:

AsyncUtils(
     () => _fooService.SaveFoo(foo),
     id => HandleFooSavedSuccessfully(id),
     ex => HandleFooSaveError(ex));

You can use the built-in tools and just write:

var task = Task.Factory.StartNew( 
                () => return _fooService.SaveFoo(foo) );
task.ContinueWith( 
                t => HandleFooSavedSuccessfully(t.Result),
                    TaskContinuationOptions.NotOnFaulted);
task.ContinueWith( 
                t => try { t.Wait(); } catch( Exception e) { HandleFooSaveError(e); },
                    TaskContinuationOptions.OnlyOnFaulted );

Granted, the last line there is a bit odd, but that's mainly because I tried to keep your existing API. If you reworked it a bit, you could simplify it...

Reed Copsey
Thanks Reed, we're currently using 3.5 but hoping to upgrade to 4. Would your advice differ knowing this?
chillitom
@Chillitom: If you can do it, I'd get the Rx framework (for .NET 3.5), as it includes a backport of the Task Parallel Lib from .NET 4. You could then use Task/Task<T> and get all of the advantages they provide. It's a free download at: http://msdn.microsoft.com/en-us/devlabs/ee794896.aspx
Reed Copsey
@Chillitom: I put in a TPL version of your code - it's not quite as succinct (though you could easily make a method to do what you do), but it's using all framework tools, plus it gives you all of the advantages of being able to wait (task.Wait()), query for completion, and leave out parts you don't need...
Reed Copsey
Good example of where `Task.ContinueWith` makes sense.
Steven Sudit
+2  A: 

Asynchronous interface (based on IAsyncResult) is useful only when you have some non-blocking call under the cover. The main point of the interface is to make it possible to do the call without blocking the caller thread.

  • This is useful in scenarios when you can make some system call and the system will notify you back when something happens (e.g. when a HTTP response is received or when an event happens).

  • The price for using IAsyncResult based interface is that you have to write code in a somewhat awkward way (by making every call using callback). Even worse, asynchronous API makes it impossible to use standard language constructs like while, for, or try..catch.

I don't really see the point of wrapping synchronous API into asynchronous interface, because you won't get the benefit (there will always be some thread blocked) and you'll just get more awkward way of calling it.

Of course, it makes a perfect sense to run the synchronous code on a background thread somehow (to avoid blocking the main application thread). Either using Task<T> on .NET 4.0 or using QueueUserWorkItem on .NET 2.0. However, I'm not sure if this should be done automatically in the service - it feels like doing this on the caller side would be easier, because you may need to perform multiple calls to the service. Using asynchronous API, you'd have to write something like:

svc.BeginGetFooId(ar1 => {
  var foo = ar1.Result; 
  foo.Prop = 123;
  svc.BeginSaveFoo(foo, ar2 => { 
    // etc...
  }
});

When using synchronous API, you'd write something like:

ThreadPool.QueueUserWorkItem(() => {
  var foo = svc.GetFooId();
  foo.Prop = 123;
  svc.SaveFoo(foo);
});
Tomas Petricek
"I don't really see the point of wrapping synchronous API into asynchronous interface," If your wrapper pushes the work onto a background thread, using IAsyncResult and the standard async pattern can give you a way to check to see if the operation has completed without blocking. There are reasons to do this, as you're effectively making the API asynchronous.
Reed Copsey
@Reed: You're right that checking whether the operation has completed is a benefit of using `IAsyncresult`, but I think that the disadvantages of the pattern (callback-based programming) are still more significant.
Tomas Petricek
@Tomas: Unless you can use .NET 4, though, there are often no better alternatives. IAsyncResult is really the only way in .NET 2.0 to give you a way to tell whether an async operation is complete, and also to propogate errors back to the calling thread...
Reed Copsey
However, you don't get any benefit from using callback-based API in this case (when the underylying call is blocking), so I'd prefer leaving the core API synchronous and just call it asynchronously (so that you can write more complicated code that uses the service).
Tomas Petricek
@Reed: I'm not saying you should not use `IAsyncResult` at all. However, I'd use it at the top-level of the call hierarchy only (and leave the inner calls synchronous). This way you can get what you need. If we use `IAsyncResult` (callback-based API) at the leafs (e.g. the API in the `IFooService`), then we have to write the code that uses the API in a callback-based way too...
Tomas Petricek
+1  A: 

The following is a response to Reed's follow-up question. I'm not suggesting that it's the right way to go.

    public static int PerformSlowly(int id)
    {
        // Addition isn't so hard, but let's pretend.
        Thread.Sleep(10000);
        return 42 + id;
    }

    public static Task<int> PerformTask(int id)
    {
        // Here's the straightforward approach.
        return Task.Factory.StartNew(() => PerformSlowly(id));
    }

    public static Lazy<int> PerformLazily(int id)
    {
        // Start performing it now, but don't block.
        var task = PerformTask(id);

        // JIT for the value being checked, block and retrieve.
        return new Lazy<int>(() => task.Result);
    }

    static void Main(string[] args)
    {
        int i;

        // Start calculating the result, using a Lazy<int> as the future value.
        var result = PerformLazily(7);

        // Do assorted work, then get result.
        i = result.Value;

        // The alternative is to use the Task as the future value.
        var task = PerformTask(7);

        // Do assorted work, then get result.
        i = task.Result;
    }
Steven Sudit
Arguably, using Lazy might make more sense if the implementation was over the thread pool queue, not Task. Task is just too clean already to benefit from further wrapping.
Steven Sudit
@Steven: The problem with Lazy is that it blocks as soon as you try to get the value. At that point, Task<T> does exactly the same thing (plus provides other benefits)... Lazy<T> is really just there for safe lazy instantiation in a multi-threaded environment.
Reed Copsey
But thanks for letting me see what you were thinking ;)
Reed Copsey
@Reed: I completely agree that Lazy makes no sense for the use case in my sample code; it's clearly redundant. What I was thinking of was Lazy as a wrapper for IAsyncResult or QueueUserWorkItem. In that case, it would avoid using a thread just to monitor another thread, as Task would. (Arguably, the alternative would be to use Task with the synchronous versions of those calls.) I do realize that the primary purpose of Lazy is JIT initialization, but I think I've at least shown that it's also a potential candidate for holding future values in those cases where Task is inappropriate.
Steven Sudit
@Steven: I'd still use task, but use Task.FromAsync. There's no reason to use QueueUserWorkItem with Task, though - since QUWI doesn't return a value at all (it's fire-and-forget, really). Should be replaced by Task.
Reed Copsey
@Reed: I assume you mean TaskFactory.FromAsync, but thanks for mentioning it; it hadn't occurred to me, yet it would be ideal. As for QUWI, back in the dark ages before 4.0, I've used it with a closure to emulate the sort of functionality that's built into Task. Task does it better, though. In fact, I think it's fair to say that we should port forward to Task whenever possible.
Steven Sudit
@Steven: Yeah - that's what I meant.
Reed Copsey