Inconsistent Reload Method Signatures between DataDropDownGrid and PagedDataBoundComponent #205

Closed
opened 2026-01-29 17:33:20 +00:00 by claunia · 6 comments
Owner

Originally created by @Mike-E-angelo on GitHub (Sep 24, 2021).

Describe the bug
The Reload method signature for the RazdenDropDownDataGrid differs from the one defined in PagedDataBoundComponent.

https://github.com/radzenhq/radzen-blazor/blob/master/Radzen.Blazor/RadzenDropDownDataGrid.razor#L224:

public void Reload()

https://github.com/radzenhq/radzen-blazor/blob/master/Radzen.Blazor/PagedDataBoundComponent.cs#L79:

public async virtual Task Reload()

Additionally, the RazdenDropDownDataGrid now calls GetAwaiter().GetResult() which can lead to thread starvation and synchronous deadlocks.

To Reproduce
NA

Expected behavior
Methods signatures are consistent across all controls. Additionally, Radzen does not lead to deadlocking and/or thread starvation whenever control methods are invoked.

Screenshots
NA

Additional context
Thank you for any consideration towards addressing this issue. 🙏

Originally created by @Mike-E-angelo on GitHub (Sep 24, 2021). <!-- IMPORTANT: Read this first!!! 1. If you own a Radzen Professional or Еnterprise subscription you can report your issue or ask us a question via email at info@radzen.com. Radzen staff will reply within 24 hours (Professional) or 16 hours (Enterprise) 2. The Radzen staff guarantees a response to issues in this repo only to paid subscribers. 3. If you have a HOW TO question start a new forum thread in the Radzen Community forum: https://forum.radzen.com. Radzen staff will close issues that are HOWTO questions. 4. Please adhere to the issue template. Specify all the steps required to reproduce the issue or link a project which reproduces it easily (without requiring extra steps such as restoring a database). --> **Describe the bug** The `Reload` method signature for the `RazdenDropDownDataGrid` differs from the one defined in `PagedDataBoundComponent`. https://github.com/radzenhq/radzen-blazor/blob/master/Radzen.Blazor/RadzenDropDownDataGrid.razor#L224: ``` public void Reload() ``` https://github.com/radzenhq/radzen-blazor/blob/master/Radzen.Blazor/PagedDataBoundComponent.cs#L79: ``` public async virtual Task Reload() ``` Additionally, the `RazdenDropDownDataGrid` [now calls](https://github.com/radzenhq/radzen-blazor/commit/9184b038b364f4de143fe70b27cc5b812c8c97e8) `GetAwaiter().GetResult()` which can lead to [thread starvation and synchronous deadlocks](https://stackoverflow.com/a/34549714). **To Reproduce** NA **Expected behavior** Methods signatures are consistent across all controls. Additionally, Radzen does not lead to deadlocking and/or thread starvation whenever control methods are invoked. **Screenshots** NA **Additional context** Thank you for any consideration towards addressing this issue. 🙏
Author
Owner

@enchev commented on GitHub (Sep 24, 2021):

Hey @Mike-E-angelo,

Changing void to task will be a breaking change, still we can do that however I'm afraid that I don't have other solution for deadlocks. Let me know if you have a better idea.

@enchev commented on GitHub (Sep 24, 2021): Hey @Mike-E-angelo, Changing void to task will be a breaking change, still we can do that however I'm afraid that I don't have other solution for deadlocks. Let me know if you have a better idea.
Author
Owner

@Mike-E-angelo commented on GitHub (Sep 24, 2021):

Dare I suggest introducing RadzenDropDownDataGrid.ReloadAsync, @enchev? 😇 If breaking changes are a concern, that would be the best course of action IMO. Then, all major data components would have the same signature for reloading data, but only one would have a different name. However, none of them would no longer introduce the possibility of deadlocks. This is, of course, a far better/desired scenario where even one introduces deadlocks.

Additionally, I would Obsolete RadzenDropDownDataGrid.Reload and point consumers to the new method.

@Mike-E-angelo commented on GitHub (Sep 24, 2021): Dare I suggest introducing `RadzenDropDownDataGrid.ReloadAsync`, @enchev? 😇 If breaking changes are a concern, that would be the best course of action IMO. Then, all major data components would have the same signature for reloading data, but only one would have a different name. However, none of them would no longer introduce the possibility of deadlocks. This is, of course, a far better/desired scenario where even one introduces deadlocks. Additionally, I would `Obsolete` `RadzenDropDownDataGrid.Reload` and point consumers to the new method.
Author
Owner

@enchev commented on GitHub (Sep 24, 2021):

As I said we are ready to change directly Reload() method to async Task even it is a breaking change however there are other places that still will need GetAwaiter().GetResult(). For example invoke of an async method from property setter.

@enchev commented on GitHub (Sep 24, 2021): As I said we are ready to change directly Reload() method to async Task even it is a breaking change however there are other places that still will need GetAwaiter().GetResult(). For example invoke of an async method from property setter.
Author
Owner

@Mike-E-angelo commented on GitHub (Sep 24, 2021):

Ah! I misread your previous statement. Yes, I would prefer Task Reload (without Async) so I accept your breaking change and we have shared understanding now. :)

Now, as for your other problem/question. What I do (and I admit it's a bit kludgy but works) is when the property is assigned, I set a flag/property of some sort that an asynchronous operation must occur. Then in OnParametersSetAsync I subsequently check on that flag/property and perform the asynchronous operation there (or return the task as the result of OnParametersSetAsync).

Here is an example of that.

You can see in the Content property that I set the flag/properties that demark asynchronous activity. I then create the Task in OnParametersSet, then return the task in OnParametersSetAsync.

Upon looking at this now, I could further improve this by setting the Task in the Content assignment. The reason why I am doing this is in OnParametersSet is that previous versions of DetermineSubject also used another property that has since been removed. The value of that property may or may not have been set by the time the property assignment occurred, and would lead to exceptions. Moving that to OnParametersSet/Async ensured the property was set/available so it is more dependable to do that there.

@Mike-E-angelo commented on GitHub (Sep 24, 2021): Ah! I misread your previous statement. Yes, I would prefer `Task Reload` (without `Async`) so I accept your breaking change and we have shared understanding now. :) Now, as for your other problem/question. What I do (and I admit it's a bit kludgy but works) is when the property is assigned, I set a flag/property of some sort that an asynchronous operation must occur. Then in `OnParametersSetAsync` I subsequently check on that flag/property and perform the asynchronous operation there (or return the task as the result of `OnParametersSetAsync`). [Here is an example of that](https://github.com/DragonSpark/Framework/blob/projects/Starbeam/DragonSpark.Presentation/Components/Content/ResultingContentView.razor). You can see [in the `Content` property](https://github.com/DragonSpark/Framework/blob/projects/Starbeam/DragonSpark.Presentation/Components/Content/ResultingContentView.razor#L35-L37) that I set the flag/properties that demark asynchronous activity. I then [create the `Task`](https://github.com/DragonSpark/Framework/blob/projects/Starbeam/DragonSpark.Presentation/Components/Content/ResultingContentView.razor#L67) in `OnParametersSet`, then [return the task](https://github.com/DragonSpark/Framework/blob/projects/Starbeam/DragonSpark.Presentation/Components/Content/ResultingContentView.razor#L71) in `OnParametersSetAsync`. Upon looking at this now, I could further improve this by setting the `Task` in the `Content` assignment. The reason why I am doing this is in `OnParametersSet` is that previous versions of `DetermineSubject` also used another property that has since been removed. The value of that property may or may not have been set by the time the property assignment occurred, and would lead to exceptions. Moving that to `OnParametersSet/Async` ensured the property was set/available so it is more dependable to do that there.
Author
Owner

@enchev commented on GitHub (Sep 27, 2021):

Fixed in 84ec51da54

@enchev commented on GitHub (Sep 27, 2021): Fixed in https://github.com/radzenhq/radzen-blazor/commit/84ec51da54fa6ccaaec09e3940f7515b13f85801
Author
Owner

@Mike-E-angelo commented on GitHub (Sep 27, 2021):

Thank you so very much @enchev! Your efforts here are greatly appreciated. 🙏

@Mike-E-angelo commented on GitHub (Sep 27, 2021): Thank you so very much @enchev! Your efforts here are greatly appreciated. 🙏
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/radzen-blazor#205