For a quite long time now I have been using async controllers from futures library. As you can imagine I was very pleased to see that finally it has been moved into primary mvc assembly, but when the day came that I tried to upgrade, I was faced with disappoitment.
Of three async patterns existing within futures assembly (begin/end, event, delegate) only one was present (slightly modified event pattern) and it was not the one I embraced (delegate pattern).
For months I have been using this simple implementation to run my database calls in parallel.
public abstract class AsyncControllerBase : AsyncController {
protected AsyncControllerBase() {
AsyncErrors = new List<Exception>();
}
protected IList<Exception> AsyncErrors { get; set; }
protected override void OnActionExecuted(ActionExecutedContext filterContext) {
if (AsyncErrors.Any())
throw new AsyncException("An operation that has been executed asynchronously failed."
+ " See the inner exception for more information.", AsyncErrors.First());
}
protected void Async(Action action) {
AsyncManager.OutstandingOperations.Increment();
ThreadPool.QueueUserWorkItem(state => {
try {
action();
} catch (Exception ex) {
AsyncErrors.Add(ex);
} finally {
AsyncManager.OutstandingOperations.Decrement();
}
});
}
}
This way I could build controller actions nice and clean via closures within single method.
public Func<ActionResult> MemberEvents(string name, int? subsetId) {
var model = AsyncCreateCommonGuildModel<GuildViewModel>();
Async(() => model.Member = BrowseRepository.GetMember(GuildId, name));
Async(() => model.Subset = BrowseRepository.GetSubset(GuildId, subsetId));
Async(() => model.Events = BrowseRepository.GetMemberEvents(GuildId, name, subsetId));
return () => {
if (model.Member == null)
return NotFound(string.Format("Member '{0}' not found.", name));
return View(model);
};
}
But now, for each async action method, I am stuck with two methods looking like this.
public void MemberEventsAsync(string name, int? subsetId) {
var model = AsyncCreateCommonGuildModel<GuildViewModel>();
Async(() => model.Member = BrowseRepository.GetMember(GuildId, name));
Async(() => model.Subset = BrowseRepository.GetSubset(GuildId, subsetId));
Async(() => model.Events = BrowseRepository.GetMemberEvents(GuildId, name, subsetId));
AsyncManager.Parameters["model"] = model;
}
public ActionResult MemberEventsCompleted(GuildViewModel model) {
if (model.Member == null)
return NotFound(string.Format("Member '{0}' not found.", RouteData.Values["name"]));
return View(model);
}
Even worse completed method wont accept action arguments, so they have to be read directly from RouteData.Values, passed via view model or AsyncManager.Parameters. Also I have been using strongly typed redirection action result
which now has to trim "Async" suffix, so very not nice!
So my question is, will delegate async pattern come back or at least will there be any usefull extension points within async action invoker classes that allow to add other patterns without the need to duplicate tons of private mvc source code?
Maciej
Don't forget to click "Mark as Answer" on the post(s) that helped you.
There are currently no plans to reintroduce the other two patterns before MVC 2 releases. This decision is driven by the fact that most of the feedback we've received is in support of the event pattern rather than the delegate or APM patterns, so we've
focused our limited resources on implementing and supporting that pattern. It also encourages testability of actions, which is important to many of the MVC developers from whom we've received comments.
If you want to support the delegate or APM patterns in your own application, you'll have to subclass the invoker. Specifically, override the AsyncControllerActionInvoker.GetControllerDescriptor() or FindAction() methods. Then that action descriptor can
use your custom implementation of BeginExecute() / EndExecute(). This will involve some duplication of our internal code, but it shouldn't be too bad.
We're currently looking at ways of making our extensibility points easier to use and require less code or less duplication. This would be broad across MVC and not necessarily confined to the invoker. But this is a longer-term goal and isn't something we're
likely to get to before MVC 2 ships.
I've also opened a bug in our internal tracking database regarding propagating XxxAsync() action method parameters to the XxxCompleted() method. It doesn't look like your sample would take advantage of this, but it sounds like a good request nonetheless.
Marked as answer by molesinski on Dec 03, 2009 08:58 PM
I must say I see the reason why delegate pattern is lacking when it comes to testability and that event pattern is vastly superior in that matter. In fact its enough to convince me to migrate to event pattern without hesitation and never look back
.
I am glad to hear about propagating parameters, that could really be handy.
Also I think there is another bug in current asynchronous pattern implementation. Apparently AsyncManager.Parameters dictionary is not thread safe. So this code can randomly fail.
Correct, but this is not a bug. The AsyncManager.Parameters property is not guaranteed to be thread-safe by definition. It's an IDictionary<,>, and that type doesn't expose the necessary methods to do the proper fine-grained locking that a thread-safe
implementation might require. Since we don't know what kind of synchronization you require, the synchronization is up to user code.
This is why the AsyncManager.Sync() method is provided. All callbacks passed to that method are serialized, so any callbacks executing within that method won't encounter thread safety issues. Alternatively, if you find AsyncManager.Sync() a bit too heavyweight
(since it also sets up the thread principal, cultures, etc.), you could just lock on the dictionary itself before modifying it. Contention-free locks are very, very inexpensive operations.
Marked as answer by molesinski on Dec 04, 2009 06:26 PM
Okay then, I assumed it might be an issue after lecture of
this article, but I guess it still "This topic is pre-release documentation and is subject to change in future release." issue.
Don't forget to click "Mark as Answer" on the post(s) that helped you.
This can also be written as an extension method in your code to eliminate lock { } duplication everywhere and make the code look cleaner (untested code):
This can also be written as an extension method in your code to eliminate lock { } duplication everywhere and make the code look cleaner (untested code):
I like this idea, at the moment I am using lock { }, but this way it should be cleaner.
Anyway, when I reported this as a bug before, just thought that since you encourage assigning asynchronous methods return values to AsyncManager.Parameters dictionary, you might want to replace generic dictionary with thread safe version, ensuring safe insertions.
Don't forget to click "Mark as Answer" on the post(s) that helped you.
We do encourage it, but within the callbacks of operations executed using the event-based asynchronous pattern (see the
Performing Multiple Operations in Parallel section of
http://msdn.microsoft.com/en-us/library/ee728598(VS.100).aspx). These will automatically be serialized and you don't have to worry about thread-safety issues.
The pattern you are using more closely resembles the Working with the BeginMethod/EndMethod Pattern section at the bottom of that link. We explicitly call out that AsyncManager.Parameters is not thread-safe and the user is responsible for serializing
access to that object. Though now that I read the documentation a bit more closely, it does have some errors in it. I'll contact our UE team to correct them.
Marked as answer by molesinski on Dec 04, 2009 07:39 PM
molesinski
Member
314 Points
71 Posts
Missing async action patterns in Mvc 2 beta
Dec 03, 2009 06:32 AM|LINK
Hello everyone,
For a quite long time now I have been using async controllers from futures library. As you can imagine I was very pleased to see that finally it has been moved into primary mvc assembly, but when the day came that I tried to upgrade, I was faced with disappoitment. Of three async patterns existing within futures assembly (begin/end, event, delegate) only one was present (slightly modified event pattern) and it was not the one I embraced (delegate pattern).
For months I have been using this simple implementation to run my database calls in parallel.
public abstract class AsyncControllerBase : AsyncController { protected AsyncControllerBase() { AsyncErrors = new List<Exception>(); } protected IList<Exception> AsyncErrors { get; set; } protected override void OnActionExecuted(ActionExecutedContext filterContext) { if (AsyncErrors.Any()) throw new AsyncException("An operation that has been executed asynchronously failed." + " See the inner exception for more information.", AsyncErrors.First()); } protected void Async(Action action) { AsyncManager.OutstandingOperations.Increment(); ThreadPool.QueueUserWorkItem(state => { try { action(); } catch (Exception ex) { AsyncErrors.Add(ex); } finally { AsyncManager.OutstandingOperations.Decrement(); } }); } }This way I could build controller actions nice and clean via closures within single method.
public Func<ActionResult> MemberEvents(string name, int? subsetId) { var model = AsyncCreateCommonGuildModel<GuildViewModel>(); Async(() => model.Member = BrowseRepository.GetMember(GuildId, name)); Async(() => model.Subset = BrowseRepository.GetSubset(GuildId, subsetId)); Async(() => model.Events = BrowseRepository.GetMemberEvents(GuildId, name, subsetId)); return () => { if (model.Member == null) return NotFound(string.Format("Member '{0}' not found.", name)); return View(model); }; }But now, for each async action method, I am stuck with two methods looking like this.
public void MemberEventsAsync(string name, int? subsetId) { var model = AsyncCreateCommonGuildModel<GuildViewModel>(); Async(() => model.Member = BrowseRepository.GetMember(GuildId, name)); Async(() => model.Subset = BrowseRepository.GetSubset(GuildId, subsetId)); Async(() => model.Events = BrowseRepository.GetMemberEvents(GuildId, name, subsetId)); AsyncManager.Parameters["model"] = model; } public ActionResult MemberEventsCompleted(GuildViewModel model) { if (model.Member == null) return NotFound(string.Format("Member '{0}' not found.", RouteData.Values["name"])); return View(model); }Even worse completed method wont accept action arguments, so they have to be read directly from RouteData.Values, passed via view model or AsyncManager.Parameters. Also I have been using strongly typed redirection action result
return RedirectTo<BrowseController>(o => o.MemberEvents("John", null))which now has to trim "Async" suffix, so very not nice!
So my question is, will delegate async pattern come back or at least will there be any usefull extension points within async action invoker classes that allow to add other patterns without the need to duplicate tons of private mvc source code?
Maciej
levib
Star
7702 Points
1099 Posts
Microsoft
Re: Missing async action patterns in Mvc 2 beta
Dec 03, 2009 07:24 PM|LINK
Hi molesinski - thanks for the feedback.
There are currently no plans to reintroduce the other two patterns before MVC 2 releases. This decision is driven by the fact that most of the feedback we've received is in support of the event pattern rather than the delegate or APM patterns, so we've focused our limited resources on implementing and supporting that pattern. It also encourages testability of actions, which is important to many of the MVC developers from whom we've received comments.
If you want to support the delegate or APM patterns in your own application, you'll have to subclass the invoker. Specifically, override the AsyncControllerActionInvoker.GetControllerDescriptor() or FindAction() methods. Then that action descriptor can use your custom implementation of BeginExecute() / EndExecute(). This will involve some duplication of our internal code, but it shouldn't be too bad.
We're currently looking at ways of making our extensibility points easier to use and require less code or less duplication. This would be broad across MVC and not necessarily confined to the invoker. But this is a longer-term goal and isn't something we're likely to get to before MVC 2 ships.
I've also opened a bug in our internal tracking database regarding propagating XxxAsync() action method parameters to the XxxCompleted() method. It doesn't look like your sample would take advantage of this, but it sounds like a good request nonetheless.
molesinski
Member
314 Points
71 Posts
Re: Missing async action patterns in Mvc 2 beta
Dec 04, 2009 12:23 PM|LINK
I must say I see the reason why delegate pattern is lacking when it comes to testability and that event pattern is vastly superior in that matter. In fact its enough to convince me to migrate to event pattern without hesitation and never look back
.
I am glad to hear about propagating parameters, that could really be handy.
Also I think there is another bug in current asynchronous pattern implementation. Apparently AsyncManager.Parameters dictionary is not thread safe. So this code can randomly fail.
public void SearchAsync(string q) { Async(() => AsyncManager.Parameters["members"] = BrowseRepository.SearchMembers(GuildId, q)); Async(() => AsyncManager.Parameters["events"] = BrowseRepository.SearchEvents(GuildId, q)); } public ActionResult SearchCompleted(object members, object events) { ViewData["members"] = members; ViewData["events"] = events; return View(); }Assigning values to ViewData directly, instead of AsyncManager.Parameters is not an option, as it is not safe aswell.
Maciej
levib
Star
7702 Points
1099 Posts
Microsoft
Re: Missing async action patterns in Mvc 2 beta
Dec 04, 2009 06:17 PM|LINK
Correct, but this is not a bug. The AsyncManager.Parameters property is not guaranteed to be thread-safe by definition. It's an IDictionary<,>, and that type doesn't expose the necessary methods to do the proper fine-grained locking that a thread-safe implementation might require. Since we don't know what kind of synchronization you require, the synchronization is up to user code.
This is why the AsyncManager.Sync() method is provided. All callbacks passed to that method are serialized, so any callbacks executing within that method won't encounter thread safety issues. Alternatively, if you find AsyncManager.Sync() a bit too heavyweight (since it also sets up the thread principal, cultures, etc.), you could just lock on the dictionary itself before modifying it. Contention-free locks are very, very inexpensive operations.
molesinski
Member
314 Points
71 Posts
Re: Missing async action patterns in Mvc 2 beta
Dec 04, 2009 06:25 PM|LINK
Okay then, I assumed it might be an issue after lecture of this article, but I guess it still "This topic is pre-release documentation and is subject to change in future release." issue.
levib
Star
7702 Points
1099 Posts
Microsoft
Re: Missing async action patterns in Mvc 2 beta
Dec 04, 2009 06:26 PM|LINK
This can also be written as an extension method in your code to eliminate lock { } duplication everywhere and make the code look cleaner (untested code):
public static void SetParameter(this AsyncManager asyncManager, string parameterName, object parameterValue) { IDictionary<string, object> parameters = asyncManager.Parameters; lock (parameters) { parameters[parameterName] = parameterValue; } }Usage:
molesinski
Member
314 Points
71 Posts
Re: Missing async action patterns in Mvc 2 beta
Dec 04, 2009 06:31 PM|LINK
I like this idea, at the moment I am using lock { }, but this way it should be cleaner.
Anyway, when I reported this as a bug before, just thought that since you encourage assigning asynchronous methods return values to AsyncManager.Parameters dictionary, you might want to replace generic dictionary with thread safe version, ensuring safe insertions.
levib
Star
7702 Points
1099 Posts
Microsoft
Re: Missing async action patterns in Mvc 2 beta
Dec 04, 2009 07:36 PM|LINK
We do encourage it, but within the callbacks of operations executed using the event-based asynchronous pattern (see the Performing Multiple Operations in Parallel section of http://msdn.microsoft.com/en-us/library/ee728598(VS.100).aspx). These will automatically be serialized and you don't have to worry about thread-safety issues.
The pattern you are using more closely resembles the Working with the BeginMethod/EndMethod Pattern section at the bottom of that link. We explicitly call out that AsyncManager.Parameters is not thread-safe and the user is responsible for serializing access to that object. Though now that I read the documentation a bit more closely, it does have some errors in it. I'll contact our UE team to correct them.