Skip to content

set_window_title(): find any running prompt-toolkit app on the terminal, not just two known ones #1792

Open

Description

@tleonhardt

Background

#1790 makes Cmd.set_window_title() safe to call from any thread. While a prompt-toolkit application
is drawing on the terminal, the title write is scheduled on that application's event loop, so it
can't interleave with rendering. When no application is running, the title is written and flushed
directly.

To find the running application, Cmd._get_prompt_loop() checks only two places:

  • self.active_session.app: the main prompt and read_input()/read_secret() prompts.
  • self._select_app_session.app: the select() menu. select() sets this field and clears it
    again when it finishes.

Any other prompt-toolkit application drawing on the same terminal is invisible to it. A title set from
another thread while such an app is running is written directly from that thread, which can
interleave with the app's rendering. That's the problem #1790 fixes for the two known cases.

This came up in several review rounds on #1790. It was left as a follow-up because fixing it properly
is a design change, not a small patch.

Cases that aren't covered

  • The ipy command: IPython's own prompt-toolkit application.
  • A command that calls prompt_toolkit.prompt(), choice(), or a dialog directly instead of
    read_input() or select().
  • A second Cmd instance sharing sys.stdout, whose main_session.output is the same ambient
    Vt100_Output.
  • Any future application kept alive during commands, such as a command toolbar or pager.

Each newly covered app currently needs another field like _select_app_session plus a cleanup that
writes any title left pending, as select() and _read_raw_input() have.

Possible approaches

  1. Look up the app through its AppSession, like StdoutProxy does.
    prompt_toolkit.patch_stdout.StdoutProxy._get_app_loop() keeps the AppSession that was current
    when it was created and checks app_session.app.loop. set_app() sets AppSession.app as a plain
    attribute, so other threads can read it. cmd2 could record the AppSession each prompt runs in and
    check that session's app, which covers anything run inside it. Apps that create their own
    AppSession (as select() does) would still need to be registered.
  2. Register every app run in one place. Add a context manager that records the running
    AppSession or Application for this Cmd, used everywhere cmd2 runs an app on its terminal.
    _get_prompt_loop() and the cleanup flush would both use it, replacing the special cases.
  3. A combination: option 1 for the ambient session, plus option 2 for sessions cmd2 creates.

Apps cmd2 doesn't start, such as IPython's prompt or a user's direct prompt_toolkit call, can only be
covered by option 1, and only when they run in an AppSession cmd2 can see.

Races on the direct-write path

When _get_prompt_loop() finds no running app, set_window_title() writes and flushes the title
directly on the calling thread. _window_title_lock only serializes title writes with each other.
prompt-toolkit's renderer and patch_stdout()'s writer thread don't take that lock, and
Vt100_Output.flush() joins and then resets its buffer with no lock of its own. So a direct write from
a worker thread can drop or duplicate the other writer's output, or land in the middle of a frame,
in these windows:

  1. A prompt starts between the check and the write. A worker finds no loop and releases the lock.
    The main thread then enters session.prompt(), app.loop is set, and the renderer starts writing
    its first frame. The worker's set_title() and flush() then run against the same output while
    the renderer is using it.
  2. patch_stdout() is still active after a prompt clears its loop. When the user presses Enter,
    app.loop is cleared and active_session goes back to main_session, but _read_raw_input()
    hasn't left patch_stdout() yet. That context's writer thread sees no app loop and writes queued
    stdout text directly to the output. A worker's direct title write at the same moment races it. When
    stdout is sys.stdout, both use the default app session's Vt100_Output. Flush window title updates without waiting for a prompt #1790 only moved the
    main thread's cleanup write outside patch_stdout(), not writes from other threads.

The old set_title()-based implementation had these races too, and #1790 narrows the windows. Closing
them properly also depends on knowing which app or writer owns the output at every moment, so it
belongs with the approaches above. For example, a registry entry could be set before the prompt starts
and cleared only after patch_stdout() exits, with direct writes waiting for or deferring to it.

Related

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions