Custom UI Window Shows Only After Download Completes

[xaevik]

• Platform: NET 6 Windows
• UI: WPF

We upgraded one of our applications from NetSparkle 2.0.11 to 2.1.2 and noticed that our Custom UI implementation only shows after the download has completed. This is similar to the issue you ran into with the Avalonia implementation.

The only appreciable difference we noticed was the following change:

--- a\NetSparkle-2.0.11\src\NetSparkle\SparkleUpdater.cs    2022-03-03 21:26:15.000000000 -0400
+++ b\NetSparkle-2.1.2\src\NetSparkle\SparkleUpdater.cs 2022-07-10 12:14:00.000000000 -0400
@@ -949,59 +966,66 @@
             if (ProgressWindow != null)
             {
                 ProgressWindow.DownloadProcessCompleted -= ProgressWindowCompleted;
                 UpdateDownloader.DownloadProgressChanged -= ProgressWindow.OnDownloadProgressChanged;
                 ProgressWindow = null;
             }
+            Action<object> showSparkleDownloadUI = (state) => {};
             if (ProgressWindow == null && UIFactory != null && !IsDownloadingSilently())
             {
-                if (!IsDownloadingSilently() && ProgressWindow == null)
-                {
                     // create the form
-                    Action<object> showSparkleDownloadUI = (state) =>
+                showSparkleDownloadUI = (state) =>
                     {
                         ProgressWindow = UIFactory?.CreateProgressWindow(this, castItem);
                         if (ProgressWindow != null)
                         {
                             ProgressWindow.DownloadProcessCompleted += ProgressWindowCompleted;
                             UpdateDownloader.DownloadProgressChanged += ProgressWindow.OnDownloadProgressChanged;
                             if (shouldShowAsDownloadedAlready)
                             {
                                 ProgressWindow?.FinishedDownloadingFile(true);
-                                _syncContext.Post((state2) => OnDownloadFinished(null, new AsyncCompletedEventArgs(null, false, null)), null);
+                            _syncContext.Post((state2) =>
+                            {
+                                OnDownloadFinished(null, new AsyncCompletedEventArgs(null, false, null));
+                                _actionToRunOnProgressWindowShown?.Invoke();
+                                _actionToRunOnProgressWindowShown = null;
+                            }, null);
                             }
                         }
                     };
+            }
                     Thread thread = new Thread(() =>
                     {
                         // call action
                         if (ShowsUIOnMainThread)
                         {
                             _syncContext.Post((state) =>
                             {
                                 showSparkleDownloadUI(null);
+                        _actionToRunOnProgressWindowShown?.Invoke();
+                        _actionToRunOnProgressWindowShown = null;
                                 ProgressWindow?.Show(ShowsUIOnMainThread);
                             }, null);
                         }
                         else
                         {
                             showSparkleDownloadUI(null);
+                    _actionToRunOnProgressWindowShown?.Invoke();
+                    _actionToRunOnProgressWindowShown = null;
                             ProgressWindow?.Show(ShowsUIOnMainThread);
                         }
                     });
 #if NETFRAMEWORK
                     thread.SetApartmentState(ApartmentState.STA);
 #else
                     if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
                     {
                         thread.SetApartmentState(ApartmentState.STA); // only supported on Windows
                     }
 #endif
                     thread.Start();
-                }
-            }
         }

         private async void ProgressWindowCompleted(object sender, DownloadInstallEventArgs args)
         {
             if (args.ShouldInstall)
             {

Shouldn't ProgressWindow?.Show(...) happen before _actionToRunOnProgressWindowShown? Or are we misunderstanding the flow of behavior here? In either case, we have locked the dependency to 2.0.11 for now. We are not sure if this was caused by the implementation of changes for Avalonia or the _actionToRunOnProgressWindowShown Action as that was implemented sometime after 2.0.11 as we had attempted every other 2.0.x release greater than patch 11 and encountered the same issue.

[Deadpikle]

Shouldn't ProgressWindow?.Show(...) happen before _actionToRunOnProgressWindowShown?

I...this would make logical sense...yes....especially given the naming...

The question is, what was I thinking when I made this commit for 2.1.0: https://github.com/NetSparkleUpdater/NetSparkle/commit/a1564b3940382daafbf08083149ccb3b9e5a0705#diff-71704bdbb8b643c1f8bbe97b710477f1745e756994fa1c05f9cea4e6693d39c3L996-L1007 where I clearly move the progress window showing from before _actionToRunOnProgressWindowShown to after?

This is where someone looks at me very sternly and with great disappointment for not documenting what I was fixing better. Clearly, I was doing something, but I don't recall what.

I'll take a look unless someone beats me to it. Sorry about that. 😬

[xaevik]

Haha no worries, it just looked a little odd when I was stepping through the code. For awhile I thought we had created it unintentionally with our ShellService. I may be able to take a deeper look in about 2 weeks when a major feature rollout we're going through is done. It's not a show stopper since we can revert to a prior release and lock it to prevent renovatebot from having a nervous breakdown.

[Deadpikle]

Here's probably what I was trying to fix before. When you put them in their proper order (show progress window -> action to show progress window), WPF doesn't work when ShowsUIOnMainThread is false:

// in CreateAndShowProgressWindow() with the move of `ProgressWindow?.Show`
LogWriter.PrintMessage("Showing download UI, progress window (non-_syncContext)");
showSparkleDownloadUI(null);
LogWriter.PrintMessage("Showing progress window");
ProgressWindow?.Show(ShowsUIOnMainThread); // this blocks on WPF due to Dispatcher.Run()
LogWriter.PrintMessage("Done showing progress window");
LogWriter.PrintMessage("About to invoke _actionToRunOnProgressWindowShown; is it null? {0}",
    _actionToRunOnProgressWindowShown == null ? "Yes" : "No");
_actionToRunOnProgressWindowShown?.Invoke();
_actionToRunOnProgressWindowShown = null;

The ProgressWindow?.Show call blocks due to the System.Windows.Threading.Dispatcher.Run() in BaseWindow.ShowWindow(isOnMainThread), so then the _actionToRunOnProgressWindowShown is never called until the progress window is closed, which of course is very counter-intuitive.

I think one fix could be that there would be some sort of message from the progress window that it had been shown before the dispatcher call (still fairly ugly), but doing this without a breaking change is proving to be a challenge. There's probably some better way. The Action was initially created to always make sure the progress window was shown before the download started, but if there is a scenario where the progress window being shown blocks any other code from running.....(Yes, there is some refactoring that could be done to make the whole UI/non-UI situation better, but that's a huge rewrite that is far beyond my capacity of time at the moment.)

A workaround would be to set ShowsUIOnMainThread = true on the SparkleUpdater instance.

[xaevik]

A workaround would be to set ShowsUIOnMainThread = true on the SparkleUpdater instance.

So we actually did try this and it had no effect. However that most likely is an issue on our side rather than with SparkleUpdater. There were some threading issues discovered in the routine that this is called from that were discovered today.

Once that threading issue is fixed we can try setting that option again and see what happens.

[xaevik]

So just as an update, we resolved the threading issue we were having where this routine was happening in our implementation. We updated back to 2.1.2 from 2.0.11 and ran it again with ShowsUIOnMainThread to true. What we see is the following behavor:

1. Application Start
2. Splash Screen Shows
3. Executes SparkleUpdater.CheckForUpdatesQuitely()
4. Executes SparkleUpdater.InitAndBeginDownload due to UpdateStatus.UpdateAvailable
5. IDownloadProgress.CreateProgressWindow() is called

Now what is supposed to happen is that IDownloadProgress.Show() should be called to show our custom UI overlay ontop of the splash screen indicating a download of an update is in progress. What appears to be happening is that _actionToRunOnProgressWindow is ran first regardless of the ShowsUIOnMainThread setting. Only then, after the download completes does IDownloadProgress.Show() get called which shows an inconsistent UI experience (progress bar is indeterminate due to no data) and the application suddenly restarts.

So here is the thing though with our implementation. Our ShellService which is responsible for initializing the application, showing a splash screen (which allows running any necessary bootstrapping functions) and then the MainWindow all runs in the background through various ValueTask methods set as .ConfigureAwait(false) as it is an external library purpose built for our thick client apps. For items that require running on the UI Thread we make intentional calls to Dispatcher.Invoke() through an IDispatcherService object which may be why we were never affected prior to the > 2.0.11 refactor but are now seeing this behavior.

We'll continue to do some investigation but it is hard to tell whether this is a bug in your library or a bug in our implementation (aren't threads fun?).

[Deadpikle]

Hi, sorry for my lack of response. Life is very busy right now (as it probably is for all of us) and I can only work on this in my free time.

I did try some things out on a separate branch here (https://github.com/NetSparkleUpdater/NetSparkle/tree/fix/race-conditions-progress-window), but the fix is rather ugly with an event being tossed between the UI and the main library. I'm not sure what else can be done to fix this properly outside of a major version bump for breaking changes. This would fix it for anyone who didn't have a custom UI, though, I think...need to test more on WinForms/Avalonia.

Threads are very fun, yes 🤪 I'll be the first to admit that there are async/threading improvements that could be made to improve this lib, but I'm reticent to make breaking changes right now with the long long wait that there was for 2.x in the first place.

Hopefully we can figure out some solution, even if it's ugly, to keep everyone moving forward with some sort of workable solution.

[xaevik]

It's all good. I was traveling for business so I had very little time to work on code as it was. Depending on my schedule this week I'll take a glance and let you know the outcome.

[xaevik]

@Deadpikle just as an update for you, I haven't really had a full chance to investigate into this or your branch yet. I am hoping to have some time soon however depending on how my next sprint goes.

[xaevik]

@Deadpikle Just wanted to loop you in that internally we're not going to be investigating this anymore. We are migrating away from WPF and over to Blazor WASM so the requirement for NetSparkle is being dropped.