chihirobelmo/FalconBMS-Alternative-Launcher

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