Temporarily fix GridViewDataSet sync deadlocks. #481
Conversation
|
I don't understand this PR. Why is this necessary? Async code should not cause deadlocks unless you are waiting for the returned task later. |
| } | ||
| else if (OnLoadingDataAsync != null) | ||
| { | ||
| return await OnLoadingDataAsync(CreateGridViewDataSetLoadOptions()); | ||
| return Task.FromResult(Task.Run(() => OnLoadingDataAsync(CreateGridViewDataSetLoadOptions())).Result); |
There was a problem hiding this comment.
Sorry, but I have no idea why this s*** can fix something.
|
@djanosik But that's exactly what GridView does when it calls DataBind(). It refreshes its content thus calling OnLoadingDataAsync, which then causes a deadlock, because the task waits for the caller to return the context while the caller waits for the task to return the result. (Now that I think about it the Task.FromResult before the Task.Run is probably redundant. It gets waited later anyway though.) |
|
I see. GridView calls GridViewDataSet.RequestRefresh() and it can indeed cause deadlocks. @exyi Is there any reason why lifetime events in controls are not async? |
|
The control lifecycle methods are not async, because in one postback there is A LOT of invocations of them and having all of them async would cause significant performance overhead. The other reason is that I don't think that it's a good idea to put significant (-> async) logic into controls, you should just operate on data from view model. Unfortunately we have redesigned GridView to not follow this, so it causes the problems. And when you really have to do that, it should not be a big deal to pause the worker thread and wait for the Task. Not a big deal from a performance point of view, but it's a problem when you have the stupid SynchronizationContext on the old ASP.NET platform. |
|
That's a good point. Maybe the GridView should not request refresh in DataBind. Maybe it should just use data that are currently loaded in DataSet. It can request refresh when something is changed. |
|
We will merge this PR to 1.1.0. But we need to find another solution. |
Fixes deadlocks that occur when GridView synchronously calls asynchronous code that accesses a database from OnLoad/OnPreRender.
How about we add async version of OnInit, OnLoad, etc.?