Skip to content

Kill the local plugin watcher with the shell via pdeathsig - #11385

Open
chadmandoo wants to merge 2 commits into
omacom:quattrofrom
chadmandoo:plugin-watcher-pdeathsig
Open

chadmandoo wants to merge 2 commits into
omacom:quattrofrom
chadmandoo:plugin-watcher-pdeathsig

Conversation

@chadmandoo

@chadmandoo chadmandoo commented Sep 11, 2026 •

Copy link
Copy Markdown

Closes #11383.

PluginRegistry spawned the local-plugin watcher as a bare Process, so an exit that skips destructors left it running: Qt leaves through _exit() when the Wayland connection fails, raising no signal. The inotifywait was reparented to systemd --user and held an inotify instance for the rest of the session. A deliberate omarchy-restart-shell does stop it cleanly — I measured an unpatched shell across a restart and it left no orphan — so this is the crash path specifically.

Enough of them exhaust fs.inotify.max_user_instances (1024 per UID), after which every inotify_init1() in the session fails with EMFILE. The error then surfaces in whatever application next asks for a watch, with nothing pointing back at the shell. On my machine a lock-path crash loop relaunched the shell 4437 times and stranded 979 watchers, taking the user to 1025 of 1024 instances; Alacritty was the first thing to refuse to start, with "too many open files".

The fix

shell/plugins/clipboard/Clipboard.qml already solves this, and says so:

Reap watchers left behind by a previous shell instance, then start our own. The pdeathsig on the watchers makes the kernel kill them whenever the shell exits, however it exits, so no further lifecycle management.

This applies the same setpriv --pdeathsig TERM idiom to the plugin watcher, which was the only long-lived Process child under shell/ still missing it.

Testing

Verified through Quickshell's real spawn path (Quickshell 0.3.1, Omarchy 4.0.3) with a minimal config, killing the shell with SIGKILL so no destructor runs:

child after kill -9 of the shell
before survives — reparented to systemd --user
after dies with the shell

Also checked for regressions in the watcher itself: inotifywait under setpriv still reports create and close_write events normally, and the child was still alive after 12 seconds with the shell running, so PDEATHSIG isn't firing early on the spawning thread.

test/shell.d/plugins-test.sh gains an assertion mirroring the two in clipboard-test.sh. It fails on the unpatched tree and passes with the change.

Ran the whole test/shell.d suite: 237 test files, 3 with failures — config-test.sh, snapper-test.sh and unowned-system-paths-test.sh, all three asking for an omarchy-pkgs checkout I don't have. They fail identically on an unmodified tree.

Not addressed here

The crash loop that exposed this looks like Quickshell's, not Omarchy's — the shell dies in lock-surface creation when the session is locked and the compositor has no valid Wayland output (Could not create EGL surface (EGL error 0x3000), then The Wayland connection experienced a fatal error: Invalid argument), and that string isn't in this repo. The watcher should not outlive the shell regardless of what kills it.

One related observation for whoever picks that up: the relaunch limiter in bin/omarchy-launch-shell allows 5 relaunches per 60-second window and resets the window on expiry, so a loop slower than 5/minute runs indefinitely inside the limit. Mine crashed about every 13 seconds and never tripped it.

🤖 Generated with Claude Code

The plugin watcher ran as a bare Process, so an exit that skips destructors left
it behind. Qt leaves through _exit() when the Wayland connection fails, raising
no signal, and the inotifywait was then reparented to `systemd --user` where it
held an inotify instance for the rest of the session. A deliberate
omarchy-restart-shell does stop it cleanly; the crash path does not.

Enough of them exhaust fs.inotify.max_user_instances, 1024 per UID, after which
every inotify_init1() in the session fails with EMFILE. The error then surfaces
in whatever application next asks for a watch, with no hint of where it came
from. A lock-path crash loop relaunched the shell 4437 times here and stranded
979 watchers, taking the user to 1025 of 1024 instances; Alacritty was the first
thing to refuse to start.

The clipboard watchers already solve this with setpriv --pdeathsig TERM, so this
applies the established idiom to the plugin watcher and asserts it in
plugins-test.sh the same way clipboard-test.sh does.

Closes omacom#11383

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

@johnpippett johnpippett left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I recommend this change based on the automated checks below. This review used Codex assistance.

Source commit: f67f285d18e5a4c9800ebaabee44c258cc544f2a.
Base commit: 31bd80daa4613ffdee995ac27467fce5a2990806.

In the earlier isolated tests, the base watcher remained alive after an abrupt parent failure. The proposed watcher exited after the same failure.

Four process lifecycle cases used real inotifywait and setpriv processes with a test parent process. Both revisions delivered file events and stopped after a clean parent shutdown. After parent SIGKILL, the base watcher remained alive, but the proposed watcher exited.

The focused plugin tests returned status 0 on both commits. The new source assertion failed against the base, as expected. Private Bubblewrap namespaces kept all process signals inside the test environment. The source remained read-only.

Documentation note, which does not block this approval: the comment in PluginRegistry.qml includes omarchy-restart-shell among exits that skip cleanup. Your PR description says a deliberate restart stopped the unpatched watcher correctly. Please remove the restart claim from this comment and describe exits that skip destructors instead. This will keep the comment consistent with your reported results.

No real Quickshell crash or graphical session ran in these independent tests. The full shell suite did not complete, so this review makes no full-suite claim. These results address watcher cleanup, not the underlying shell crash.

Before submission, the source and base commits still matched these results. The tests were not repeated for this submission.

@calebl

calebl commented Sep 22, 2026

Copy link
Copy Markdown

Independent reproduction on different hardware, with numbers that support the mechanism described here.

System: Beelink SER (AMD Ryzen 7 6800U, amdgpu), Omarchy 4.0.0 (omarchy-dev 4.0.0.r2155.gf2cf3ce), Quickshell 0.3.1, kernel 7.2.5-4-omarchy. Single output (HDMI-A-1).

I went looking for a RAM problem and found 398 orphaned inotifywait processes, all reparented to systemd --user, all watching ~/.config/omarchy/plugins:

$ ps -o ppid= -p $(pgrep -d, -f inotifywait) | sort | uniq -c | sort -rn
    398 1116   (systemd --user)
      1 3453442 (quickshell)   <- the live one

The orphan count tracks the crash count almost exactly:

Measure Count
The Wayland connection experienced a fatal error in journal 406
Orphaned inotifywait 398
fs.inotify.max_user_instances consumed ~463 of 1024

406 crashes, 398 orphans. I was about 45% of the way to the EMFILE wall you describe, without any symptom that pointed at the shell.

No coredump exists for any of those 406 exits. coredumpctl has unrelated entries only. That is consistent with your account of Qt leaving through _exit() and raising no signal, which is exactly why no destructor runs to stop the child.

On the crash path itself, my journal shows the trigger is output loss during lock-surface creation:

09:20:48  idle-monitor: active
09:21:03  WARN: attempted to use dangling screen object   (x9)
09:21:04  WARN quickshell.hyprland.ipc: Got removal for monitor "FALLBACK" which was not previously tracked.
09:21:04  WARN: The Wayland connection experienced a fatal error: Invalid argument
09:21:04  Omarchy shell exited with status 255; relaunching.
09:21:06  omarchy lock  lock-stranded: recovering
09:21:06  omarchy lock  lock-requested
09:21:06  omarchy lock  lock-pending: screen-stabilizing
09:21:20  WARN: attempted to use dangling screen object   <- next iteration

with INFO qt.qpa.wayland: There are no outputs - creating placeholder screen preceding it. On a single-output machine the idle timeout locks the session and powers the display off, so the lock path runs against a placeholder QScreen(name=""). The stranded-lock recovery on relaunch re-enters the same path, which is what sustains the loop.

It is a race rather than a certainty — Got removal for monitor "FALLBACK" appears 1770 times against 406 fatal errors, so roughly 23% of output-loss events go fatal.

Your note about the relaunch limiter is correct in practice. My crashes are 17-18s apart, about 3.5/minute, comfortably under omarchy-launch-shell's 5-per-60s. My longest unbroken loop ran 2026-09-18 12:35:34 to 18:17:20 — 5h42m, roughly 395 crashes — and never tripped the limiter.

The setpriv --pdeathsig TERM change would have contained all of it. +1 from me.

omarchy-restart-shell stops the shell cleanly and its watcher exits with it; on an unpatched shell a restart left no orphan, while kill -9 did. The comment named the restart among the exits that skip destructors, which would send the next reader after the wrong path.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@omarchybot

Copy link
Copy Markdown
Collaborator

Reviewed at ca954a0 against quattro at 31bd80d. The fix holds: I reproduced the leak and saw this change stop it.

What ran, on a disposable Omarchy worker VM:

  • Base shell, kill -9 on quickshell: its plugin inotifywait was reparented to systemd --user and kept running, and the relaunched shell started a second one. With this branch linked as the running shell, the same kill -9 took the watcher down with it, and only the new shell's watcher was left.
  • With the shell running, the watcher under setpriv was still alive after 15 seconds, so the pdeathsig does not fire early. Its stdout is still the pipe to quickshell, and setpriv --pdeathsig TERM inotifywait … reports create and close_write as before.
  • test/shell.d/plugins-test.sh passes on the head, and with PluginRegistry.qml reverted to base its new assertion fails. clipboard-test.sh passes (73/73) and ./test/cli is green. The full ./test/shell run had 3227 passes and 2 failures, in ascii-test.sh ("Hi is 18 columns wide") and branding-about-animation-test.sh. Both fail the same way on unpatched base, and this diff touches neither.

Pushed to your branch: ca954a0 corrects the code comment only. It listed omarchy-restart-shell among the exits that skip destructors, but on the worker an unpatched shell restarted that way stopped its watcher cleanly, matching your description. The comment now names the _exit(), crash and SIGKILL paths. The command and the test are unchanged.

Related: this is the fix for #7150. #11383 was closed as a duplicate of it, so the Closes #11383 here will not close the open issue. I tried to add the link and omabot refused, because the push above reset this pull request's classification and it has not been re-confirmed. #7230 also claimed #7150 and is closed. I found no other open pull request changing this watcher's lifetime.

Who checked it: Claude Opus 5.5 only. The second opinion (Codex Medium) did not run: the reviewer answers a ping, but omabot refused the review because today's review budget is spent. So this is one model's review, and the item is not marked verified until the second opinion has run on this head. It is waiting on that review, then on the maintainer. Nothing is needed from you.

@greptile-apps

greptile-apps Bot commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 4/5

[Medium risk] Changes how the plugin watcher process is spawned.

The PR appears safe to merge, with a non-blocking gap in automated crash-path coverage.

Findings

  1. P2 Crash behavior remains untested ▶

Summary

The PR starts the local-plugin inotifywait watcher with a parent-death signal so it exits when the shell dies without running destructors.

  • Adds a source-level assertion for the new command.
  • The assertion does not provide an automated regression check for the crash behavior the PR addresses.

Reviews (1) · Last reviewed commit: "Drop the restart claim from the plugin w..."


const registrySource = fs.readFileSync(path.join(root, 'shell/services/PluginRegistry.qml'), 'utf8')
check(
/command: \[\s*"setpriv",\s*"--pdeathsig",\s*"TERM",\s*"inotifywait"/.test(registrySource),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Crash behavior remains untested The new assertion checks only that the source contains setpriv --pdeathsig TERM. It cannot catch a regression where the plugin watcher starts but survives a shell crash, leaving the resource leak this PR addresses undetected. A lifecycle test that kills the watcher's owner and checks that the watcher exits would cover this behavior, as the clipboard watcher test does.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

@omarchybot omarchybot removed the bug Something isn't working label Oct 2, 2026
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.

Plugin watcher outlives a shell crash, leaking an inotifywait each time until inotify instances are exhausted

4 participants