Skip to content

fix(colors): only send terminal escape sequences to real terminals - #3622

Open
M7MMAD-OMAR wants to merge 1 commit into
end-4:mainfrom
M7MMAD-OMAR:fix/pty-escape-sequence-leak
Open

fix(colors): only send terminal escape sequences to real terminals#3622
M7MMAD-OMAR wants to merge 1 commit into
end-4:mainfrom
M7MMAD-OMAR:fix/pty-escape-sequence-leak

Conversation

@M7MMAD-OMAR

Copy link
Copy Markdown

Describe your changes

apply_anyterm writes the generated OSC palette into every /dev/pts/*.

Not every pty belongs to a terminal emulator. A process can hold one open
without using it as a controlling terminal, and on a setup with KDE components
installed — which the Fedora dependency set pulls in — kded6 does exactly
that. What lands there is not a recolour; it is a desktop notification whose
body is the raw escape sequence. That is what #3453 is reporting, with the
screenshot of colour codes in a notification popup.

Here, /dev/pts/2 is held by kded6 and has no controlling-terminal process,
while every real terminal's pty has one:

/dev/pts/2   controlling=[]              holders=[kded6]     <- notification
/dev/pts/8   controlling=[fish]          holders=[fish]
/dev/pts/9   controlling=[bash]          holders=[bash]

The same loop also leaks processes. Each write is a disowned cat redirected
into the pty, and on a pty nobody reads from, it never finishes. On this machine
71 of them had accumulated, one batch per wallpaper change.

The fix

Skip any pty that has no process using it as a controlling terminal. That is
exactly the property separating a terminal emulator's pty from one that is
merely held open, and it needs no allowlist of terminal names to maintain.

Measured on the same machine, per wallpaper change:

before after
leaked cat processes 3 0
notification from kded6 yes no

Fixes #3453

Is it ready? Questions/feedback needed?

Ready. Tested on Fedora 44 with Hyprland 0.56.2 and quickshell-git 0.2.1;
open terminals still recolour on a wallpaper change.

Two notes:

  • ps -t is in coreutils/procps and already assumed elsewhere in the repo, so
    this adds no dependency.
  • The workaround in the issue thread filters with fuser + ps comm against a
    list of terminal names. The controlling-terminal check gets the same result
    without anyone having to keep that list current.

apply_anyterm writes the generated OSC palette into every /dev/pts/*. Not
every pty belongs to a terminal emulator: a process can hold one open without
using it as a controlling terminal, and on a setup with KDE components
installed kded6 does exactly that. What arrives there is not a recolour, it is
a desktop notification whose body is the raw escape sequence -- which is what
end-4#3453 is reporting.

The same loop leaks processes. Each write is a disowned `cat` redirected into
the pty, and on a pty nobody is reading it never finishes. On this machine 71
of them had accumulated, one batch per wallpaper change.

Skip any pty that has no process using it as a controlling terminal. That is
the property that separates a terminal emulator's pty from one that is merely
held open, and it needs no allowlist of terminal names.

Before/after on the same machine, per wallpaper change:

  leaked cat processes    3 -> 0
  notification from kded6 yes -> no

Fixes end-4#3453
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Changing Wallpaper Causes System Notication

1 participant