search

grumpydev / TinyIoC

An easy to use, hassle free, Inversion of Control Container for small projects, libraries and beginners alike.
MIT License
830 stars 235 forks source link

Fix Double Publish with PublishAsync #149

Closed gillima closed 3 years ago

gillima commented 3 years ago

Messages are published twice but the callback is never invoked when using PublishAsync. Fixes #148

niemyjski commented 3 years ago

Thanks for the PR! I left some comments :)

gillima commented 3 years ago

Ups, just got reminded by the compiler that async void methods without await are running synchronously. Therefor I changed to a non-async void method that starts a not awaited task...

This async method lacks 'await' operators and will run synchronously. Consider using the 'await' operator to await non-blocking API calls, or 'await Task.Run(...)' to do CPU-bound work on a background thread.

gkarabin commented 3 years ago

Ups, just got reminded by the compiler that async void methods without await are running synchronously. Therefor I changed to a non-async void method that starts a not awaited task...

This async method lacks 'await' operators and will run synchronously. Consider using the 'await' operator to await non-blocking API calls, or 'await Task.Run(...)' to do CPU-bound work on a background thread.

This probably fixes a different behavior change, in that .net framework would not observe any exceptions when using BeginInvoke. EndInvoke, which wasn’t called, would be needed for that. Awaiting the Task would cause exceptions to be observed. This most recent change returns that behavior.

gillima commented 3 years ago

Awaiting the Task would cause exceptions to be observed

The exception would be "visible" inside the PublishAsync method but not for the calling method. The issue is that you can't await async void methods. Without making PublishAsync returning a Task I don't think there's a way to get the exception outside of PublishAsync

gkarabin commented 3 years ago

Right. I think that the behavior on your PR allows NETSTANDARD/NETCORE apps to match the behavior of .NET Framework prior to #132. #132 was a mess (with respect to the TinyMessenger change).

niemyjski commented 3 years ago

Merged! Thanks again for the pr! If I could get some help with https://github.com/grumpydev/TinyIoC/issues/136 that would be a massive help for pushing a release with this.