Skip to content

Commit 1a6a556

Browse files
committed
Stage 3 review: clear passes through when unbound, like the rest
The pass-through guard reached every intercepted method except this one. A retained delegating wrapper therefore had a retired bridge take its lock and invalidate its state on someone else's behalf -- and taking the lock is not harmless: the caller may hold a higher-level lock, and the ordering rule forbids that nesting, so acting as owner turned someone else's clear into an exception. The test could not have caught it. It asserted the delegated call returned, which it did whenever nothing else held a lock. It now asserts what the guard is actually for: no terminal transaction is taken, under a higher-level lock, and the retired bridge's own state is left alone.
1 parent 8f745a2 commit 1a6a556

2 files changed

Lines changed: 38 additions & 3 deletions

File tree

‎cmd2/prompt_toolkit_bridge.py‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -546,7 +546,14 @@ def _clear_through_bridge(self) -> None:
546546
the remembered origin is forgotten rather than carried across -- recovery would
547547
otherwise place the next frame where the prompt used to be. Cursor reports already in
548548
flight describe the screen before the clear and are discarded with it.
549+
550+
Unbound, this passes straight through, for the reason given on the render wrapper: a
551+
retired bridge is not the terminal's owner, and taking its lock or invalidating its
552+
state on someone else's behalf would be acting as one.
549553
"""
554+
if not self._bound:
555+
self._originals["clear"]()
556+
return
550557
self._last_emission_committed = False
551558
with self._lock.transaction("clear"):
552559
try:

‎tests/test_prompt_toolkit_bridge.py‎

Lines changed: 31 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@
3232
ReservedModeFailureError,
3333
)
3434
from cmd2.terminal_display import TerminalDisplay
35-
from cmd2.terminal_transaction import TerminalLock, current_transaction
35+
from cmd2.terminal_transaction import HigherLevelLock, TerminalLock, current_transaction
3636

3737

3838
class TtyStringIO(io.StringIO):
@@ -1514,7 +1514,13 @@ def tracing_fire() -> None:
15141514
assert fired == [1]
15151515
assert handled == [1]
15161516

1517-
def test_delegated_erase_and_clear_still_work_after_unbinding(self) -> None:
1517+
def test_delegated_erase_and_clear_take_no_transaction_after_unbinding(self) -> None:
1518+
"""A retired bridge is not the terminal's owner and must not act as one.
1519+
1520+
Taking its lock is not harmless: the caller may hold a higher-level lock, and the
1521+
ordering rule forbids that nesting -- so acting as owner turns someone else's clear
1522+
into an exception.
1523+
"""
15181524
harness = self.bound()
15191525
harness.renderer.request_absolute_cursor_position = lambda: None # type: ignore[method-assign]
15201526
erase, clear = harness.renderer.erase, harness.renderer.clear
@@ -1528,11 +1534,33 @@ def passing_clear() -> None:
15281534
harness.renderer.erase = passing_erase # type: ignore[method-assign]
15291535
harness.renderer.clear = passing_clear # type: ignore[method-assign]
15301536
harness.bridge.unbind()
1537+
harness.bridge.require_resynchronization("before the delegated calls")
15311538

1532-
with set_app(harness.app):
1539+
harness.stream_recorder.transactions.clear()
1540+
with set_app(harness.app), HigherLevelLock("routing"):
15331541
harness.renderer.erase()
15341542
harness.renderer.clear()
15351543

1544+
assert harness.stream_recorder.transactions
1545+
assert all(state is None for state in harness.stream_recorder.transactions)
1546+
1547+
def test_a_delegated_clear_does_not_invalidate_the_retired_bridge(self) -> None:
1548+
"""Its state describes a terminal it no longer owns; changing it means nothing."""
1549+
harness = self.bound()
1550+
harness.renderer.request_absolute_cursor_position = lambda: None # type: ignore[method-assign]
1551+
harness.bridge.set_prompt_anchor(7)
1552+
clear = harness.renderer.clear
1553+
1554+
def passing_clear() -> None:
1555+
clear()
1556+
1557+
harness.renderer.clear = passing_clear # type: ignore[method-assign]
1558+
harness.bridge.unbind()
1559+
1560+
with set_app(harness.app):
1561+
harness.renderer.clear()
1562+
assert harness.bridge.prompt_anchor == 7
1563+
15361564
def test_delegated_cursor_reports_still_work_after_unbinding(self) -> None:
15371565
harness = self.bound()
15381566
request = harness.renderer.request_absolute_cursor_position

0 commit comments

Comments
 (0)