Prevent zombie Blooms and restore trying many ports (BL-15551)

src/BloomExe/FatalExceptionHandler.cs

@@ -82,7 +82,7 @@ protected void HandleTopLevelError(object sender, ThreadExceptionEventArgs e)
             {
                 //Are we inside a Application.Run() statement?
                 if (Application.MessageLoop)
-                    Application.Exit();
+                    Program.Exit();
                 else
                     Environment.Exit(1); //the 1 here is just non-zero
             }

src/BloomExe/InstallerSupport.cs

@@ -217,7 +217,7 @@ internal static void MakeBloomRegistryEntries(string[] programArgs)
                 );
                 ProblemReportApi.ShowProblemDialog(null, exception, "", "fatal");
                 // Not sure these lines are reachable. Just making sure.
-                Application.Exit();
+                Program.Exit();
                 return;
             }
 

src/BloomExe/MiscUI/BrowserProgressDialog.cs

@@ -120,7 +120,7 @@ public static void DoWorkWithProgressDialog(
                 dlg.ShowDialog(owner); // returns when dialog closed
                 if (progress.HasFatalProblemBeenReported)
                 {
-                    Application.Exit();
+                    Program.Exit();
                 }
 
                 ProgressDialogApi.SetCancelHandler(null);
@@ -260,7 +260,7 @@ public static void HandleProgressDialogClosed(ApiRequest request)
         {
             if (_progress?.HasFatalProblemBeenReported ?? false)
             {
-                Application.Exit();
+                Program.Exit();
             }
 
             _doWhenProgressDialogCloses?.Invoke();

src/BloomExe/Program.cs

@@ -11,6 +11,7 @@
 using System.Threading;
 using System.Threading.Tasks;
 using System.Windows.Forms;
+using Bloom.Api;
 using Bloom.CLI;
 using Bloom.Collection;
 using Bloom.Collection.BloomPack;
@@ -103,7 +104,7 @@ static class Program
         static int Main(string[] args1)
         {
             // AttachConsole(-1);	// Enable this to allow Console.Out.WriteLine to be viewable (must run Bloom from terminal, AFAIK)
-            bool gotUniqueToken = false;
+            _gotUniqueToken = false;
             _uiThreadId = Thread.CurrentThread.ManagedThreadId;
             Logger.Init();
             // Configure TempFile to create temp files with a "bloom" prefix so we can
@@ -404,7 +405,7 @@ static int Main(string[] args1)
                                         BloomMessageBox.ShowInfo(msg);
                                         return 1;
                                     }
-                                    gotUniqueToken = true;
+                                    _gotUniqueToken = true;
 
                                     if (
                                         projectContext.TeamCollectionManager.CurrentCollection
@@ -470,7 +471,7 @@ static int Main(string[] args1)
                         // and now we need to get the lock as usual before going on to load the new collection.
                         if (!UniqueToken.AcquireToken(_mutexId, "Bloom"))
                             return 1;
-                        gotUniqueToken = true;
+                        _gotUniqueToken = true;
                     }
                     else if (IsBloomBookOrder(args))
                     {
@@ -480,7 +481,7 @@ static int Main(string[] args1)
                         {
                             // No other instance isrunning. Start up normally (and show the book just downloaded).
                             // See https://silbloom.myjetbrains.com/youtrack/issue/BL-3822
-                            gotUniqueToken = true;
+                            _gotUniqueToken = true;
                         }
                         else if (forEdit)
                         {
@@ -501,7 +502,7 @@ static int Main(string[] args1)
                             // control key is held down so allow second instance to run; note that we're deliberately in this state
                             if (UniqueToken.AcquireTokenQuietly(_mutexId))
                             {
-                                gotUniqueToken = true;
+                                _gotUniqueToken = true;
                             }
                             else
                             {
@@ -511,7 +512,7 @@ static int Main(string[] args1)
                         else if (UniqueToken.AcquireToken(_mutexId, "Bloom"))
                         {
                             // No other instance is running. We own the token and should release it on quitting.
-                            gotUniqueToken = true;
+                            _gotUniqueToken = true;
                         }
                         else
                         {
@@ -642,8 +643,7 @@ static int Main(string[] args1)
             {
                 // Check memory one final time for the benefit of developers.  The user won't see anything.
                 //Bloom.Utils.MemoryManagement.CheckMemory(true, "Bloom finished and exiting", false);
-                if (gotUniqueToken)
-                    UniqueToken.ReleaseToken();
+                ReleaseBloomToken();
 
                 _sentry?.Dispose();
                 // In a debug build we want to be able to see if we're leaving garbage around. (Note: this doesn't seem to be working.)
@@ -652,7 +652,7 @@ static int Main(string[] args1)
                 // - we might delete something in use by the instance that has the token
                 // - we would delete the token itself (since _mutexid and NamePrefix happen to be the same),
                 // allowing a later duplicate process to start normally.
-                if (gotUniqueToken)
+                if (_gotUniqueToken)
                     TempFile.CleanupTempFolder();
 #endif
             }
@@ -661,6 +661,16 @@ static int Main(string[] args1)
             return 0;
         }
 
+        /// <summary>
+        /// Normally only used as Main() cleans up on exit.
+        /// Also in forced shutdown on timeout.
+        /// </summary>
+        public static void ReleaseBloomToken()
+        {
+            if (_gotUniqueToken)
+                UniqueToken.ReleaseToken();
+        }
+
         private static bool IsWebviewMissingOrTooOld()
         {
             string version;
@@ -909,7 +919,7 @@ public static void RestartBloom(bool hardExit, string args = null)
                 Thread.Sleep(2000);
                 if (hardExit || _projectContext?.ProjectWindow == null)
                 {
-                    Application.Exit();
+                    Program.Exit();
                 }
                 else
                 {
@@ -1286,6 +1296,69 @@ private static bool OpenProjectWindow(string collectionPath)
             return false;
         }
 
+        private static Thread _forceShutdownThread;
+
+        public static void Exit()
+        {
+            EnsureBloomReallyQuits();
+            Application.Exit();
+        }
+
+        private static void EnsureBloomReallyQuits()
+        {
+            if (_forceShutdownThread != null)
+                return; // already started
+            // We've had a problem with Bloom failing to shutdown, leaving a zombie thread that prevents
+            // other instances from starting up. If shutdown takes more than 20 seconds, we will just
+            // forcefully exit.
+            _forceShutdownThread = new Thread(() =>
+            {
+                Thread.Sleep(20000);
+                // We hope to never get here; it means that Bloom failed to fully
+                // quit 20s after we called Application.Exit(). We will now do some
+                // minimal essential cleanup and then force an exit.
+                // It might be nice to tell the user, but since we don't know
+                // what went wrong or where we are in the shutdown process, it's not
+                // obvious what to say. I don't even see how we can reliably show a message,
+                // since we're about to force a shut down. And I don't think there's
+                // anything that will be left in a bad state that the user might need
+                // to deal with.
+                try
+                {
+                    Logger.WriteEvent("Forcing Bloom to close after normal shutdown timed out.");
+                }
+                catch (Exception)
+                {
+                    // We might have already shut down the logger. If we can't log it, too bad.
+                }
+
+                // These things MUST be done so that Bloom can be started again without problems.
+                // They should be very fast.
+                try
+                {
+                    BloomServer._theOneInstance?.CloseListener();
+                }
+                catch (Exception)
+                {
+                    // Anything that goes wrong here shouldn't prevent either trying
+                    // the next cleanup or exiting.
+                }
+
+                try
+                {
+                    Program.ReleaseBloomToken();
+                }
+                catch (Exception) { }
+
+                Environment.Exit(1);
+                // If that doesn't prove drastic enough, an even more forceful option is
+                // Process.GetCurrentProcess().Kill();
+            });
+            _forceShutdownThread.Priority = ThreadPriority.Highest;
+            _forceShutdownThread.IsBackground = true; // so it won't block shutdown
+            _forceShutdownThread.Start();
+        }
+
         private static void HandleProjectWindowActivated(object sender, EventArgs e)
         {
             _projectContext.ProjectWindow.Activated -= HandleProjectWindowActivated;
@@ -1415,7 +1488,7 @@ public static bool ChooseACollection(Shell formToClose = null)
                             // and we don't want to exit the application. Otherwise, we are in initial startup and
                             // closing the chooser should exit the application.
                             if (formToClose == null)
-                                Application.Exit();
+                                Program.Exit();
                             return false;
                         }
 
@@ -1480,11 +1553,11 @@ static void HandleProjectWindowClosed(object sender, EventArgs e)
             }
             else if (((Shell)sender).QuitForVersionUpdate)
             {
-                Application.Exit();
+                Program.Exit();
             }
             else
             {
-                Application.Exit();
+                Program.Exit();
             }
         }
 
@@ -1734,6 +1807,7 @@ private static bool IsSameActualLanguage(string code1, string code2)
 
         private static bool _errorHandlingHasBeenSetUp;
         private static IDisposable _sentry;
+        private static bool _gotUniqueToken;
 
         /// ------------------------------------------------------------------------------------
         internal static void SetUpErrorHandling()

src/BloomExe/ProjectContext.cs

@@ -374,7 +374,7 @@ IContainer parentContainer
                     MessageBoxButtons.OK,
                     MessageBoxIcon.Error
                 );
-                Application.Exit();
+                Program.Exit();
             }
 
             var server = parentContainer.Resolve<BloomServer>();

src/BloomExe/web/BloomServer.cs

@@ -1334,7 +1334,7 @@ public virtual void EnsureListening()
                 ErrorReport.NotifyUserOfProblem(GetServerStartFailureMessage());
                 Logger.WriteEvent("Error: Could not start up internal HTTP Server");
                 Analytics.ReportException(new ApplicationException("Could not start server."));
-                Application.Exit();
+                Program.Exit();
             }
 
             Logger.WriteEvent("Server will use " + ServerUrlEndingInSlash);
@@ -1387,18 +1387,9 @@ private bool AttemptToOpenPort()
 
         private bool HandleExceptionOpeningPort(Exception error)
         {
-            if (!Program.RunningUnitTests && !Program.RunningSecondInstance)
-                NonFatalProblem.Report(
-                    ModalIf.None,
-                    PassiveIf.Alpha,
-                    "Could not open " + ServerUrlEndingInSlash,
-                    "Could not start server on that port",
-                    error
-                );
-            else
-                Console.WriteLine(
-                    $"Cannot open {ServerUrlEndingInSlash}: {error.Message} ({error.GetType().Name})"
-                );
+            Console.WriteLine(
+                $"Cannot open {ServerUrlEndingInSlash}: {error.Message} ({error.GetType().Name})"
+            );
             try
             {
                 if (_listener != null)
@@ -1437,7 +1428,7 @@ private static void VerifyWeAreNowListening()
             catch (Exception error)
             {
                 ErrorReport.NotifyUserOfProblem(error, GetServerStartFailureMessage());
-                Application.Exit();
+                Program.Exit();
             }
 
             ServerIsListening = true;
@@ -2216,19 +2207,7 @@ var kvp in _workers.Where(kvp =>
                             }
                         }
 
-                        // stop listening for incoming http requests
-                        Debug.Assert(_listener.IsListening);
-                        if (_listener.IsListening)
-                        {
-                            //In BL-3290, a user quitely failed here each time he exited Bloom, with a Cannot access a disposed object.
-                            //according to http://stackoverflow.com/questions/11164919/why-httplistener-start-method-dispose-stuff-on-exception,
-                            //it's actually just responding to being closed, not disposed.
-                            //I don't know *why* for that user the listener was already stopped.
-                            _listener.Stop();
-                        }
-                        //if we keep getting that exception, we could move the Close() into the previous block
-                        _listener.Close();
-                        _listener = null;
+                        CloseListener();
                     }
                     if (_cache != null)
                     {
@@ -2252,6 +2231,29 @@ var kvp in _workers.Where(kvp =>
             IsDisposed = true;
         }
 
+        /// <summary>
+        /// Close the listener and free up the port. Normally called only by Dispose.
+        /// Also used when normal shutdown times out.
+        /// </summary>
+        public void CloseListener()
+        {
+            if (_listener == null)
+                return; // probably called from shutdown timer, and normal shutdown got this far
+            // stop listening for incoming http requests
+            Debug.Assert(_listener.IsListening);
+            if (_listener.IsListening)
+            {
+                //In BL-3290, a user quitely failed here each time he exited Bloom, with a Cannot access a disposed object.
+                //according to http://stackoverflow.com/questions/11164919/why-httplistener-start-method-dispose-stuff-on-exception,
+                //it's actually just responding to being closed, not disposed.
+                //I don't know *why* for that user the listener was already stopped.
+                _listener.Stop();
+            }
+            //if we keep getting that exception, we could move the Close() into the previous block
+            _listener.Close();
+            _listener = null;
+        }
+
         #endregion
     }
 

src/BloomExe/web/BloomWebSocketServer.cs

@@ -161,7 +161,7 @@ WebSocketServer server
                 Environment.NewLine,
                 server.Port
             );
-            Application.Exit();
+            Program.Exit();
         }
 
         /// <summary>