Broken async implementation to fetch RSS (in develop branch)
arithex opened this issue · 1 comments
I was investigating some non-determinsitic AL crashes on startup.. from logs there appears to be some nondeterministic execution in the MainWindow.Loaded codepath.
https://forum.falcon-bms.com/post/380034
I produced a one-off build of the 'develop' branch, which includes the new change to make RSSReader async. It generates a few compiler warnings, and it doesn't appear to have helped.. so I took a closer look.
There's a lot to unpack here -- certainly the goal to fetch RSS (and also the auto-update XML) async'ly is good.. but it needs to be done via BackgroundWorker and Dispatcher.BeginInvoke. I don't think async/await is going to help here, because the RSSReader.Read() method isn't actually async (doesn't call any downstream async methods or await on anything). This generates a burst of compiler warnings, that the code block will execute sync'ly and the Task.WaitAll() will do nothing.
I also think the idea of subscribing to 'Loaded' event on your own window instance, is needlessly awkward -- listeners to this event are invoked up in the base class Window.OnInitialized() .. so your app's subclass of MainWindow hasn't had opportunity to finalize initializing its tree of child elements.
I have a PR ready that replaces the Loaded event handler with a simple override of OnInitialized() virtual method.. and replaces the problematic async-void stuff with a simple call to ThreadPool.QueueUserWorkItem() to fetch RSS, and then Dispatcher.Invoke to update the UI back on the UI thread.
I will clean it up and send it for review..
fixed in pull/107