updated fix2375 here only to see what CI says about it

.github/workflows/test.yml

@@ -172,4 +172,6 @@ jobs:
           password: raspberry
           port:  ${{ job.services.rpios.ports[5022] }}
           command_timeout: 30m
-          script: xvfb-run python make.py check
+          script: |
+            export MU_TEST_SUPPORT_SLOW_RUNNER_FLAG=1
+            xvfb-run python make.py check

mu/app.py

@@ -172,6 +172,9 @@ def excepthook(*exc_args):
     Log exception and exit cleanly.
     """
     logging.error("Unrecoverable error", exc_info=(exc_args))
+    # Very important to release shared memory used to signal an app instance is running
+    # as we are going to exit below
+    _shared_memory.release()
     if exc_args[0] != KeyboardInterrupt:
         try:
             log_file = base64.standard_b64encode(LOG_FILE.encode("utf-8"))
@@ -191,7 +194,8 @@ def excepthook(*exc_args):
                 "e": error,  # error message
             }
             args = urllib.parse.urlencode(params)
-            webbrowser.open("https://codewith.mu/crash/?" + args)
+            if "MU_SUPPRESS_CRASH_REPORT_FORM" not in os.environ:
+                webbrowser.open("https://codewith.mu/crash/?" + args)
         except Exception as e:  # The Alamo of crash handling.
             logging.error("Failed to report crash", exc_info=e)
         sys.__excepthook__(*exc_args)
@@ -200,6 +204,13 @@ def excepthook(*exc_args):
         sys.exit(0)
 
 
+def setup_exception_handler():
+    """
+    Install global exception handler
+    """
+    sys.excepthook = excepthook
+
+
 def setup_logging():
     """
     Configure logging.
@@ -231,8 +242,6 @@ def setup_logging():
         stdout_handler.setFormatter(formatter)
         stdout_handler.setLevel(logging.DEBUG)
         log.addHandler(stdout_handler)
-    else:
-        sys.excepthook = excepthook
 
 
 def setup_modes(editor, view):
@@ -275,15 +284,17 @@ class SharedMemoryMutex(object):
     NAME = "mu-tex"
 
     def __init__(self):
-        self._shared_memory = QSharedMemory(self.NAME)
+        sharedAppName = self.NAME
+        if "MU_TEST_SUPPORT_RANDOM_APP_NAME_EXT" in os.environ:
+            sharedAppName += os.environ["MU_TEST_SUPPORT_RANDOM_APP_NAME_EXT"]
+        self._shared_memory = QSharedMemory(sharedAppName)
 
     def __enter__(self):
         self._shared_memory.lock()
         return self
 
     def __exit__(self, *args, **kwargs):
         self._shared_memory.unlock()
-        self._shared_memory.detach()
 
     def acquire(self):
         if self._shared_memory.attach():
@@ -293,6 +304,9 @@ def acquire(self):
             self._shared_memory.create(8)
             self._shared_memory.data()[:8] = struct.pack("q", os.getpid())
 
+    def release(self):
+        self._shared_memory.detach()
+
 
 _shared_memory = SharedMemoryMutex()
 
@@ -326,6 +340,8 @@ def run():
     then runs the application. Specific tasks include:
 
     - set up logging
+    - set up global exception handler
+    - check that another instance of the app isn't already running (exit if so)
     - create an application object
     - create an editor window and status bar
     - display a splash screen while starting
@@ -338,6 +354,8 @@ def run():
     logging.info("Platform: {}".format(platform.platform()))
     logging.info("Python path: {}".format(sys.path))
     logging.info("Language code: {}".format(i18n.language_code))
+
+    setup_exception_handler()
     check_only_running_once()
 
     #
@@ -443,5 +461,10 @@ def load_theme(theme):
     # Restore the previous session along with files passed by the os
     editor.restore_session(sys.argv[1:])
 
+    # Save the exit code for sys.exit call below.
+    exit_status = app.exec_()
+    # Clean up the shared memory used to signal an app instance is running
+    _shared_memory.release()
+
     # Stop the program after the application finishes executing.
-    sys.exit(app.exec_())
+    sys.exit(exit_status)

mu/logic.py

@@ -1428,7 +1428,7 @@ def quit(self, *args, **kwargs):
         }
         save_session(session)
         logger.info("Quitting.\n\n")
-        sys.exit(0)
+        QtCore.QCoreApplication.exit(0)
 
     def show_admin(self, event=None):
         """

mu/virtual_environment.py

@@ -473,9 +473,9 @@ def run_subprocess(self, *args, **kwargs):
         if stderr_output:
             output += "\n\nSTDERR: " + stderr_output
         logger.debug(
-            "Process returned %d; output: %s",
-            process.returncode,
-            compact(output),
+            "Process returned {}; output: {}".format(
+                process.returncode, compact(output)
+            ),
         )
         return process.returncode == 0, output
 

tests/conftest.py

@@ -1,6 +1,8 @@
 from unittest import mock
 
 import pytest
+import os
+import random
 from PyQt5.QtWidgets import QApplication
 
 from mu import settings
@@ -27,3 +29,13 @@ def disable_autosave():
         settings.SettingsBase, "register_for_autosave"
     ) as register:
         yield register
+
+
+@pytest.fixture(autouse=True)
+def temp_shared_mem_app_name():
+    """Make multi-instance execution blocking shared memory app name unique for tests"""
+    os.environ["MU_TEST_SUPPORT_RANDOM_APP_NAME_EXT"] = "_" + str(
+        random.randint(0, 100000000)
+    )
+    yield
+    os.environ.pop("MU_TEST_SUPPORT_RANDOM_APP_NAME_EXT", "")

tests/test_app.py

@@ -6,16 +6,19 @@
 import os.path
 import pytest
 import subprocess
+import time
 
 from unittest import mock
 from mu.app import (
     excepthook,
     run,
     setup_logging,
+    setup_exception_handler,
     AnimatedSplash,
     StartupWorker,
     vlogger,
     check_only_running_once,
+    _shared_memory,
 )
 from mu.debugger.config import DEBUGGER_PORT
 
@@ -189,8 +192,7 @@ def test_setup_logging_without_envvar():
     """
     Ensure that logging is set up in some way.
 
-    Resetting the MU_LOG_TO_STDOUT env var ensures that the crash handler
-    will be enabled and stdout logging not
+    Resetting the MU_LOG_TO_STDOUT env var should mean stdout logging is disabled
     """
     os.environ.pop("MU_LOG_TO_STDOUT", "")
     with mock.patch("mu.app.TimedRotatingFileHandler") as log_conf, mock.patch(
@@ -208,15 +210,14 @@ def test_setup_logging_without_envvar():
             encoding=ENCODING,
         )
         logging.getLogger.assert_called_once_with()
-        assert sys.excepthook == excepthook
 
 
 def test_setup_logging_with_envvar():
     """
     Ensure that logging is set up in some way.
 
-    Setting the MU_LOG_TO_STDOUT env var ensures that the crash handler
-    will not be enabled and stdout logging will
+    Setting the MU_LOG_TO_STDOUT env var ensures that stdout logging
+    will be enabled
     """
     os.environ["MU_LOG_TO_STDOUT"] = "1"
     with mock.patch("mu.app.TimedRotatingFileHandler") as log_conf, mock.patch(
@@ -234,7 +235,18 @@ def test_setup_logging_with_envvar():
             encoding=ENCODING,
         )
         logging.getLogger.assert_called_once_with()
-        # ~ assert sys.excepthook == excepthook
+        # clear this to avoid interfering with other tests
+        os.environ.pop("MU_LOG_TO_STDOUT", "")
+
+
+def test_setup_except_hook():
+    """
+    confirm that setup_exception_handler() is setting up the global exception hook
+    """
+    saved = sys.excepthook
+    setup_exception_handler()
+    assert sys.excepthook == excepthook
+    sys.excepthook = saved
 
 
 def test_run():
@@ -274,7 +286,9 @@ class Win(mock.MagicMock):
         "mu.app.QThread"
     ), mock.patch(
         "mu.app.StartupWorker"
-    ) as mock_worker:
+    ) as mock_worker, mock.patch(
+        "mu.app.setup_exception_handler"
+    ) as mock_set_except:
         run()
         assert set_log.call_count == 1
         # foo.call_count is instantiating the class
@@ -293,6 +307,7 @@ class Win(mock.MagicMock):
         assert ex.call_count == 1
         assert mock_event_loop.call_count == 1
         assert mock_worker.call_count == 1
+        assert mock_set_except.call_count == 1
         window.load_theme.emit("day")
         qa.assert_has_calls([mock.call().setStyleSheet(DAY_STYLE)])
         window.load_theme.emit("night")
@@ -416,7 +431,9 @@ def test_only_running_once():
     """
     If we are the first to acquire the application lock we should succeed
     """
+    setup_exception_handler()
     check_only_running_once()
+    _shared_memory.release()
     assert True
 
 
@@ -430,13 +447,108 @@ def test_running_twice():
     # process tree; otherwise the second attempt to acquire the mutex will
     # succeed (which we don't want to happen for our purposes)
     #
-    cmd1 = "import time;"
-    "from mu import app;"
-    "app.check_only_running_once();"
-    "time.sleep(2)"
-    cmd2 = "import time;"
-    "from mu import app;"
-    "app.check_only_running_once()"
-    subprocess.Popen([sys.executable, cmd1])
-    result = subprocess.run([sys.executable, cmd2])
-    assert result.returncode == 2
+    # Note:  This test involves too much timing contingent execution.
+    #
+    # IF THIS TEST FAILS:
+    #  Try increasing times (in the else clause if not on PiOS)
+    #
+    # processOneLaunchTime - time to wait before launching process two so that process
+    #                       can finish launching and create shared memory mutex buffer.
+    # processTwoLaunchAndExitTime - time for process two to launch, test, and exit.
+    #                           Process one must run at least this long before quitting.
+    #
+    # processTwoLaunchAndExitTime should always be longer than processOneLaunchTime.
+    #
+    # Set time to wait for process two to launch, extending it if
+    # env var is set (used for PiOS on CI currently - very slow)
+    if "MU_TEST_SUPPORT_SLOW_RUNNER_FLAG" in os.environ:
+        processOneLaunchTime = 1.5
+        processTwoLaunchAndExitTime = 3
+    else:
+        processOneLaunchTime = 0.5
+        processTwoLaunchAndExitTime = 1
+    cmd1 = "".join(
+        (
+            "-c",
+            "import time;",
+            "import os;",
+            "from mu import app;",
+            "print('process 1 id: {}'.format(os.getpid()));",
+            "app.setup_exception_handler();",
+            "app.check_only_running_once();",
+            "time.sleep({0});".format(processTwoLaunchAndExitTime),
+            "app._shared_memory.release()",
+        )
+    )
+    cmd2 = "".join(
+        (
+            "-c",
+            "import os;",
+            "from mu import app;",
+            "print('process 2 id: {}'.format(os.getpid()));",
+            "app.setup_exception_handler();",
+            "app.check_only_running_once()",
+        )
+    )
+
+    child1 = subprocess.Popen([sys.executable, cmd1])
+    # let child 1 fully launch first (required)
+    time.sleep(processOneLaunchTime)
+    child2 = subprocess.run([sys.executable, cmd2])
+
+    # wait for process 1 to exit _after_ launching process 2
+    # and wait until it's done to access return code
+    result1 = child1.wait()
+    result2 = child2.returncode
+
+    assert result1 == 0
+    assert result2 == 2
+
+
+# The test_running_twice_after_generic_exception() test is
+# similar enough to this that we can probably skip this one
+@pytest.mark.skip("Good test but slow")
+def test_running_twice_after_exception():
+    """
+    If we run, throw an exception and then run again we should succeed.
+    """
+    # call test that causes app to throw an exception because runs it twice
+    test_running_twice()
+    # test that we can still run after exception thrown
+    test_only_running_once()
+
+
+def test_running_twice_after_generic_exception():
+    """
+    If we run and the app throws an exception, the exception handler
+    should clean up shared memory sentinel and running again should succeed.
+    """
+    #
+    # check_only_running_once() acquires shared memory block
+    # raise uncaught exception to trigger exception handler
+    # (subprocess should exit with exit code)
+    # check to see if can run app again
+    #
+
+    # set show browser on crash suppression env for test
+    os.environ["MU_SUPPRESS_CRASH_REPORT_FORM"] = "1"
+    cmd1 = "".join(
+        (
+            "-c",
+            "import os;",
+            "from mu import app;",
+            "print('process 1 id: {}'.format(os.getpid()));",
+            "app.setup_exception_handler();",
+            "app.check_only_running_once();"
+            "raise RuntimeError('intentional test exception')",
+            # Intentionally do not manually release shared memory here.
+            # Test is testing that exception handler does this.
+        )
+    )
+
+    child1 = subprocess.run([sys.executable, cmd1])
+    assert child1.returncode == 1
+    # confirm exception handler cleared shared memory and we can still run
+    test_only_running_once()
+    # clear browser launch suppression env flag
+    os.environ.pop("MU_SUPPRESS_CRASH_REPORT_FORM", "")

tests/test_logic.py

@@ -2227,9 +2227,9 @@ def test_quit_calls_sys_exit(mocked_session):
     mock_open.return_value.write = mock.MagicMock()
     mock_event = mock.MagicMock()
     mock_event.ignore = mock.MagicMock(return_value=None)
-    with mock.patch("sys.exit", return_value=None) as ex, mock.patch(
-        "builtins.open", mock_open
-    ):
+    with mock.patch(
+        "PyQt5.QtCore.QCoreApplication.exit", return_value=0
+    ) as ex, mock.patch("builtins.open", mock_open):
         ed.quit(mock_event)
     ex.assert_called_once_with(0)