Stage 3 review: finish the deferred teardown before letting go of the refusal

A pause that timed out left the application as the running thread expected to
find it: its layout, its key bindings, its erase-on-done setting. That was the
right thing to do while the thread was alive. Once the thread ended, that state
belonged to nobody, and simply dropping the refusal handed the next prompt an
application still dressed as the command display.

The teardown the pause could not do is finished at the moment the refusal
lifts, and the refusal lifts only if it could be. Both halves of the pause --
returning the borrowed application, and handing over the keys typed meanwhile
-- are now one method, so the deferred path cannot drift from the ordinary one.

The regression asserts the layout, bindings and erase setting are back, not
just that the refusal is gone. Checking only the refusal is what let this
through.

Author: tleonhardt

cmd2/cmd2.py

@@ -2155,7 +2155,10 @@ def _require_terminal_ownership(self) -> None:
         display = self._display_holding_terminal
         if display is None:
             return
-        if not display.thread_is_alive:
+        if display.complete_abandoned_shutdown():
+            # Its thread has ended, so the teardown its timed-out pause could not do has been
+            # finished here. Clearing the reference without that would hand the next prompt an
+            # application still dressed as the command display.
             self._display_holding_terminal = None
             return
         raise RuntimeError("the bottom toolbar's display has not released the terminal")

cmd2/command_toolbar.py

@@ -454,21 +454,43 @@ def _pause(self) -> None:
                     self._thread.join(timeout=_SHUTDOWN_TIMEOUT)
                     if self._thread.is_alive():
                         self._abandon_stuck_display()
-                    else:
-                        self._thread = None
-                # Return the borrowed application to the main prompt, including on
-                # proxy failures. The upstream toolbar owned a separate application.
-                if self._display_stack is not None:
-                    self._display_stack.close()
-                    self._display_stack = None
-                # Application.run() saves its unprocessed queue before the thread
-                # exits. Those keys arrived after the ones handled by save_key().
-                pending_keys = get_typeahead(self.app.input)
-                store_typeahead(self.app.input, self._keys + pending_keys)
-                self._keys.clear()
+                self._finish_pause()
         finally:
             self._pausing = False
 
+    def _finish_pause(self) -> None:
+        """Give the borrowed application back and hand over the keys typed meanwhile.
+
+        Only safe once the display's thread has ended: until then it is still using the layout
+        and key bindings this puts back.
+        """
+        self._thread = None
+        # Return the borrowed application to the main prompt, including on proxy failures.
+        # The upstream toolbar owned a separate application.
+        if self._display_stack is not None:
+            self._display_stack.close()
+            self._display_stack = None
+        # Application.run() saves its unprocessed queue before the thread exits. Those keys
+        # arrived after the ones handled by save_key().
+        pending_keys = get_typeahead(self.app.input)
+        store_typeahead(self.app.input, self._keys + pending_keys)
+        self._keys.clear()
+
+    def complete_abandoned_shutdown(self) -> bool:
+        """Finish the teardown a timed-out pause could not do, if the thread has since ended.
+
+        A pause that gave up left the application as the running thread expected to find it:
+        its layout, its key bindings, its erase-on-done setting. Once the thread is gone that
+        state belongs to nobody, and the next prompt would otherwise render as the command
+        display.
+
+        :return: whether the display has now been fully torn down
+        """
+        if self.thread_is_alive:
+            return False
+        self._finish_pause()
+        return True
+
     def _abandon_stuck_display(self) -> None:
         """Report that the display did not stop, and refuse to pretend it did.
 

tests/test_command_toolbar.py

@@ -976,24 +976,38 @@ def test_a_surviving_display_blocks_the_prompt(toolbar_app, monkeypatch) -> None
 
 
 def test_the_refusal_lifts_when_the_display_finally_exits(toolbar_app, monkeypatch) -> None:
-    """The thread may yet finish, and the session should not stay broken if it does."""
+    """The thread may yet finish, and the session should not stay broken if it does.
+
+    Lifting the refusal is not the whole of it. The pause that timed out never restored the
+    application it had borrowed, so the prompt that comes next would render with the command
+    display's layout and key bindings unless that teardown is finished first.
+    """
     app, _, _ = toolbar_app
     blocked = threading.Event()
     monkeypatch.setattr(command_toolbar, "_STARTUP_TIMEOUT", 0.2)
     monkeypatch.setattr(command_toolbar, "_SHUTDOWN_TIMEOUT", 0.2)
 
+    prompt_layout = app.main_session.app.layout
+    prompt_bindings = app.main_session.app.key_bindings
+    prompt_erase = app.main_session.app.erase_when_done
+
     app.main_session.bottom_toolbar = lambda: blocked.wait(timeout=10) or "STATUS"
     with app._command_toolbar_context():
         pass
     surviving = app._display_holding_terminal
     assert surviving is not None
+    assert app.main_session.app.layout is not prompt_layout
 
     blocked.set()
     surviving._thread.join(timeout=5)
 
     with app.suspend_bottom_toolbar():
         pass
+
     assert app._display_holding_terminal is None
+    assert app.main_session.app.layout is prompt_layout
+    assert app.main_session.app.key_bindings is prompt_bindings
+    assert app.main_session.app.erase_when_done == prompt_erase
 
 
 def test_a_surviving_display_stops_another_from_starting(toolbar_app, monkeypatch) -> None:

tests/test_reserved_lifecycle.py

@@ -598,3 +598,7 @@ class StuckDisplay:
     """A command display whose thread will not finish."""
 
     thread_is_alive = True
+
+    def complete_abandoned_shutdown(self) -> bool:
+        """Report that the teardown cannot be finished while the thread runs."""
+        return False