I was investigating some non-determinsitic AL crashes on startup.. from logs there appears to be some nondeterministic execution in the MainWindow.Loaded codepath.
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.
arithex
commented
11 months ago
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.
https://github.com/chihirobelmo/FalconBMS-Alternative-Launcher/blob/40fd9004b3bbbc794e5733d4350fc709330c5620/Falcon%20BMS%20Alternative%20Launcher/Windows/MainWindow.xaml.cs#L56
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..