Skip to content
This repository has been archived by the owner on Dec 14, 2018. It is now read-only.

TemplateRenderer should use ThrowIfFaulted when waiting for faulted task #782

Closed
yishaigalatzer opened this issue Jul 11, 2014 · 7 comments
Assignees
Milestone

Comments

@yishaigalatzer
Copy link
Contributor

Html.EditorFor and Html.DisplayFor are synchronous today. When rendering a view for the editor \ display template, it calls Wait(). Instead we should have both a sync and async pipeline (add Html.EditorForAsync, Html.DisplayForAsync) and have the synchronous version make the blocking call.

Original discussion:

See: https://github.com/aspnet/Mvc/blob/dev/src/Microsoft.AspNet.Mvc.Core/Rendering/Html/TemplateRenderer.cs#L103

Looks like the pipeline needs to accommodation async and we shouldn't be making blocking calls.

@yishaigalatzer
Copy link
Contributor Author

@davidfowl @lodejard - We are thinking that editor templates should just be sync only.
But if you have other thoughts lets chat about it.

@pranavkm
Copy link
Contributor

pranavkm commented Aug 7, 2014

Spoke to @loudej and he thinks we should have an async rendering pipeline for Html.EditorFor \ DisplayFor. The synchronous calls will end up doing blocking calls (.Wait()) like they do now.

cc @dougbu

@danroth27 danroth27 modified the milestones: 6.0.0-alpha3, 6.0.0-alpha4 Aug 12, 2014
@yishaigalatzer
Copy link
Contributor Author

I was under the impression that wait calls can end up being deadlocks, and its generally frowned upon.

Can we instead say that editor templates are sync only?

@pranavkm
Copy link
Contributor

Spoke to @GrabYourPitchforks about this today and his opinion was that in the absence of a sync context, we shouldn't have the same sort of issues calling Wait(). His only suggestion was to change the actual Wait() call to task.GetAwaiter().GetResult(). This preserves the stack trace in the event the Task throws.

@yishaigalatzer
Copy link
Contributor Author

Should we have an extension method to do that?

-----Original Message-----
From: "Pranav K" notifications@github.com
Sent: ‎8/‎14/‎2014 3:42 PM
To: "aspnet/Mvc" Mvc@noreply.github.com
Cc: "Yishai Galatzer" yishai_galatzer@yahoo.com
Subject: Re: [Mvc] TemplateRenderer should not call renderasync().Wait()(#782)

Spoke to @GrabYourPitchforks about this today and his opinion was that in the absence of a sync context, we shouldn't have the same sort of issues calling Wait(). His only suggestion was to change the actual Wait() call to task.GetAwaiter().GetResult(). This preserves the stack trace in the event the Task throws.

Reply to this email directly or view it on GitHub.

@pranavkm pranavkm changed the title TemplateRenderer should not call renderasync().Wait() TemplateRenderer should support both sync and async code paths. Aug 14, 2014
@pranavkm
Copy link
Contributor

We decided to reduce the scope of this bug and address the issue with calling Task.Wait().

@davidfowl
Copy link
Member

Was this done? If so why is it still working? /cc @yishaigalatzer @danroth27

@pranavkm pranavkm changed the title TemplateRenderer should support both sync and async code paths. TemplateRenderer should use ThrowIfFaulted when waiting for faulted task Aug 29, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants