diff --git a/AGENTS.md b/AGENTS.md index 951a68551..de68ffdfe 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -800,6 +800,15 @@ gets the same chooser. `ui/PickerList.qml` hands `Row` `Picker.HIDDEN_COLS` and the chooser never inherits Mode or Kind from `ViewState`, whatever the header menu has switched on for the browser window. +**Stalled chooser listings.** `PickerListing.qml` owns a separate read-only backend for list, +listpaths and window. Navigation invalidates its output immediately, kills and reaps that worker, +then starts only the latest request with a fresh stdout parser. The chooser's other `Backend` +opts into `pickerOnly`: only picker checks and capability reads may be sent, and shutdown can +terminate a stuck identity check without touching a file-manager write backend. `PickerLifecycle` +waits for both workers only after the reply file has been saved. `tests/picker-stall.sh` drives +blocked FIFO reads, latest-navigation recovery, incomplete stale output, early cancellation, +failed spawn and process reaping under offscreen Quickshell; it needs no production mount. + **Recent, and why it is read-only.** `SendPicker.html` draws a Recent row above Home in the rail, says the location's own name where the path would be, and draws Parent disabled with the words "unavailable in Recent". The history it lists is the desktop's own, diff --git a/tests/picker-stall-helper.py b/tests/picker-stall-helper.py new file mode 100644 index 000000000..4f814032d --- /dev/null +++ b/tests/picker-stall-helper.py @@ -0,0 +1,35 @@ +#!/usr/bin/env python3 +"""A sequential backend blocked in a real FIFO read, with no user mount or cloud traffic.""" +import json +import os +from pathlib import Path +import signal +import sys + +root = Path(os.environ['FLEA_PICKER_FIXTURE']) +assert (root / '.flea-test-sandbox').is_file() +with (root / 'pids').open('a') as log: + log.write(f'{os.getpid()}\n') +signal.signal(signal.SIGTERM, signal.SIG_IGN) +for line in sys.stdin: + request = json.loads(line) + with (root / 'requests').open('a') as log: + log.write(line) + command = request['c'] + if command == 'list' and request['path'] == '/stalled': + print('{"t":"blocked"}', flush=True) + # Leave an incomplete reply behind: a reused stdout parser would join it to the next worker. + print('{"t":"listed","path":"/stale",', end='', flush=True) + with (root / 'stall.fifo').open() as pipe: + pipe.read() + elif command == 'picker': + print('{"t":"picker","id":1,"op":"blocked"}', flush=True) + with (root / 'stall.fifo').open() as pipe: + pipe.read() + elif command in ('list', 'listpaths'): + print(json.dumps({'t': 'listed', 'path': request.get('path', '/'), 'n': 1, 'read': 0, 'sort': 0}), flush=True) + print('{"t":"rows","start":0,"rows":[{"n":"local.txt","d":false,"s":1,"m":1,"p":33188,"i":"text-x-generic","t":false,"k":0}],"kinds":[]}', flush=True) + elif command == 'window': + print('{"t":"rows","start":1,"rows":[{"n":"next.txt"}],"kinds":[]}', flush=True) + elif command == 'quit': + break diff --git a/tests/picker-stall.qml b/tests/picker-stall.qml new file mode 100644 index 000000000..4d80e32e9 --- /dev/null +++ b/tests/picker-stall.qml @@ -0,0 +1,80 @@ +import QtQuick +import Quickshell +import "flea" as Flea + +ShellRoot { + id: root + property string scenario: Quickshell.env("FLEA_PICKER_CASE") + property bool stalled: false + property bool localReady: false + property bool paged: false + property bool validationBlocked: false + property bool stopping: false + property int failures: 0 + property double began: Date.now() + function stop() { stopping = true; lifecycle.quit() } + Flea.PickerListing { + id: listing + onFailed: function(reason) { + if (root.scenario.indexOf("missing") >= 0) root.stop() + else { console.log(reason); root.failures++; root.stop() } + } + onMessage: function(message) { + if (message.t === "blocked") { + root.stalled = true + next.start() + } else if (message.t === "listed") { + if (message.path !== "/local") root.failures++ + root.localReady = true + listing.window(1, 1) + } else if (message.t === "rows" && message.start === 1) { + root.paged = message.rows[0].n === "next.txt" + root.stop() + } + } + } + Flea.Backend { + id: checks + pickerOnly: true + onPickerResult: function(message) { if (message.op === "blocked") root.validationBlocked = true } + onFailed: function(where, input, message, mode) { + if (root.scenario.indexOf("missing") < 0) { console.log(message); root.failures++ } + } + } + Flea.PickerLifecycle { + id: lifecycle + checks: checks + listing: listing + onStopped: { + var good = root.stopping && !root.failures && Date.now() - root.began < 2000 + if (root.scenario.indexOf("missing") < 0 && root.scenario.indexOf("early") < 0) + good = good && root.stalled && root.validationBlocked + if (root.scenario === "navigate") good = good && root.localReady && root.paged + console.log("picker-stall", root.scenario, good ? "PASS" : "FAIL", Date.now() - root.began) + Qt.exit(good ? 0 : 1) + } + } + Timer { + id: next + interval: 100 + onTriggered: { + if (root.scenario === "navigate") { + listing.request({c: "list", path: "/superseded", first: 1}) + listing.request({c: "list", path: "/local", first: 1}) + } else root.stop() + } + } + Timer { + id: failedStartOrder + interval: 100 + onTriggered: { root.stopping = true; listing.quit(); checks.testFailedStartDuringQuit() } + } + Timer { interval: 3000; running: true; onTriggered: Qt.exit(1) } + Component.onCompleted: { + if (scenario === "missing-order") { failedStartOrder.start(); return } + listing.request({c: "list", path: "/stalled", first: 1}) + checks.send({c: "transfer", rows: [0], dest: "/not-allowed"}) + checks.send({c: "picker", op: "validate", id: 1}) + if (scenario.indexOf("early") === 0) stop() + } +} diff --git a/tests/picker-stall.sh b/tests/picker-stall.sh new file mode 100755 index 000000000..e2ded8cba --- /dev/null +++ b/tests/picker-stall.sh @@ -0,0 +1,77 @@ +#!/usr/bin/env bash +# A real Quickshell Process blocks on a private FIFO just as a sequential backend can block on +# FUSE. New navigation must reap it, discard its incomplete stdout line and page the replacement. +# In parallel the identity-check backend blocks on its own read: cancel must reap both, even if +# neither reads another stdin byte. Immediate cancellation covers the before-onStarted race. +set -eu +. "$(dirname "$0")/../tools/flea-sandbox-guard" +cd "$(dirname "$0")/.." +command -v qs >/dev/null +fixture="$FIXTURE_ROOT/picker-stall-$$" +sandbox_make "$fixture" +cleanup() { + # On a failed assertion the owned helper may still be blocked; never signal a recycled PID. + python3 - "$fixture" <<'PYEND' +import os +from pathlib import Path +import signal +import sys +root = Path(sys.argv[1]) +assert (root / '.flea-test-sandbox').is_file() +for text in (root / 'pids').read_text().splitlines() if (root / 'pids').exists() else []: + process = Path('/proc') / text + try: + if os.stat(process).st_uid == os.getuid() and os.fsencode(root / 'helper') in (process / 'cmdline').read_bytes().split(b'\0'): + os.kill(int(text), signal.SIGKILL) + except FileNotFoundError: + pass +PYEND + sandbox_remove "$fixture" +} +trap cleanup EXIT +mkdir -p "$fixture/flea/js" +cp ui/PickerListing.qml ui/PickerLifecycle.qml ui/Backend.qml "$fixture/flea/" +cp ui/js/Messages.js "$fixture/flea/js/" +# Drive the real runningChanged handler in the rare cancel-before-FailedToStart order. Natural +# missing-executable timing here fails before cancel; this test-only method fixes the event order, +# not the handler, so restoring the old guard makes missing-order time out. +python3 - "$fixture/flea/Backend.qml" <<'PYEND' +from pathlib import Path +import sys +path = Path(sys.argv[1]) +text = path.read_text().replace(' function send(object) {', ''' + function testFailedStartDuringQuit() { + if (child.running) throw new Error("Failed-start fixture unexpectedly has a child") + root.queueing = true + root.quitting = true + child.runningChanged() + } + function send(object) { +''', 1) +path.write_text(text) +PYEND +printf 'module flea\nsingleton ViewState 1.0 ViewState.qml\n' > "$fixture/flea/qmldir" +printf 'pragma Singleton\nimport QtQuick\nQtObject { property var state: ({}) }\n' > "$fixture/flea/ViewState.qml" +cp tests/picker-stall.qml "$fixture/shell.qml" +cp tests/picker-stall-helper.py "$fixture/helper" +chmod +x "$fixture/helper" +mkfifo "$fixture/stall.fifo" +for scenario in navigate cancel early missing early-missing missing-order; do + : > "$fixture/pids" + : > "$fixture/requests" + helper="$fixture/helper" + [[ $scenario != *missing* ]] || helper="$fixture/missing" + if ! QT_QPA_PLATFORM=offscreen FLEA_BIN="$helper" FLEA_PICKER_CASE="$scenario" FLEA_PICKER_FIXTURE="$fixture" \ + timeout 6 qs -p "$fixture/shell.qml" > "$fixture/output" 2>&1; then + cat "$fixture/output"; exit 1 + fi + grep -F "picker-stall $scenario PASS" "$fixture/output" || { cat "$fixture/output"; exit 1; } + if [[ $scenario == navigate ]] && [[ $(wc -l < "$fixture/pids") != 3 ]]; then + echo "FAIL: superseded navigation launched an extra worker"; exit 1 + fi + while read -r pid; do + if kill -0 "$pid" 2>/dev/null; then echo "FAIL: owned helper $pid survived"; exit 1; fi + done < "$fixture/pids" + if grep -q '"c":"transfer"' "$fixture/requests"; then echo 'FAIL: picker sent a write'; exit 1; fi +done +printf 'picker-stall: 6 process lifecycle scenarios passed\n' diff --git a/tests/run-all.sh b/tests/run-all.sh index 7f37e2519..e95ecfd55 100755 --- a/tests/run-all.sh +++ b/tests/run-all.sh @@ -43,6 +43,7 @@ done # Named, not run: each needs something this script cannot assume it has. One list, read twice: it # is printed here and it is what the audit below checks, so a suite cannot be quietly excluded. not_run=" +picker-stall|needs Quickshell; runs offscreen read-only worker cancellation tests ui|needs the display, and refuses beside a Flea it did not start drag|needs the display and a real pointer through uinput cardsizes|needs the display, a real pointer through uinput, and Hyprland to resize the window diff --git a/ui/Backend.qml b/ui/Backend.qml index 081779c75..daee0565a 100644 --- a/ui/Backend.qml +++ b/ui/Backend.qml @@ -101,9 +101,12 @@ Item { // A quit is in flight, so the child's exit is the end the shell asked for, not a failure. property bool quitting: false + // Only the portal chooser opts in: it has no filesystem write operations to drain. + property bool pickerOnly: false // Everything the UI sends goes through here, so the protocol has exactly one author. function send(object) { + if (root.pickerOnly && object.c !== "picker" && object.c !== "formats") return var line = JSON.stringify(object) + "\n" if (root.queueing) { root.pending.push(line) @@ -280,6 +283,12 @@ Item { return } root.quitting = true + if (root.pickerOnly) { + root.pending = [] + if (child.running) child.signal(9) + else root.quitReady() + return + } // Before the child is up a quit would only join the pending queue and never be written, // and nothing can be in flight yet, so there is nothing to drain and the shell goes now. if (root.queueing || !child.running) { @@ -337,6 +346,7 @@ Item { } onStarted: { + if (root.pickerOnly && root.quitting) { child.signal(9); return } root.queueing = false // Asked once per process: which formats exist cannot change while the backend runs. root.askFormats() @@ -351,7 +361,10 @@ Item { if (root.queueing && !child.running) { root.queueing = false root.pending = [] - root.failed("backend", "", "the backend could not be started", 0) + // FailedToStart has no exited signal. A chooser already cancelling still owes its + // lifecycle an answer; a started child clears queueing and must be reaped in exited. + if (root.pickerOnly && root.quitting) root.quitReady() + else root.failed("backend", "", "the backend could not be started", 0) } } diff --git a/ui/PickerLifecycle.qml b/ui/PickerLifecycle.qml new file mode 100644 index 000000000..d39880d28 --- /dev/null +++ b/ui/PickerLifecycle.qml @@ -0,0 +1,28 @@ +import QtQuick +import Quickshell + +// Called only after the portal reply has been saved (or the save has failed). Both processes are +// owned by this chooser; neither can write selected files, so a stalled read needs no write drain. +Item { + id: root + required property var checks + required property var listing + signal stopped() + property bool checksStopped: false + property bool listingStopped: false + property bool finished: false + function quit() { listing.quit(); checks.quit() } + function finish() { + if (finished || !checksStopped || !listingStopped) return + finished = true + stopped() + } + Connections { + target: root.checks + function onQuitReady() { root.checksStopped = true; root.finish() } + } + Connections { + target: root.listing + function onQuitReady() { root.listingStopped = true; root.finish() } + } +} diff --git a/ui/PickerListing.qml b/ui/PickerListing.qml new file mode 100644 index 000000000..d7572234b --- /dev/null +++ b/ui/PickerListing.qml @@ -0,0 +1,85 @@ +import QtQuick +import Quickshell +import Quickshell.Io + +// Listings can block inside FUSE. Only this read-only worker is replaceable; selection identities +// belong to the picker's separate backend, and the file manager's write backend is never signalled. +Item { + id: root + signal message(var value) + signal failed(string reason) + signal quitReady() + property var current: null + property var pending: null + property bool quitting: false + + function clear() { + pending = null + if (current) { + current.obsolete = true + if (current.running) current.signal(9) + } + } + function request(value) { + if (quitting || (value.c !== "list" && value.c !== "listpaths")) return + pending = value + if (current) { + current.obsolete = true + if (current.running) current.signal(9) + } else launch() + } + function launch() { + if (quitting || !pending || current) return + var value = pending + pending = null + current = worker.createObject(root, {request: value}) + current.running = true + } + function window(start, count) { + if (current && current.running && !current.obsolete && !quitting) + current.write(JSON.stringify({c: "window", start: start, count: count}) + "\n") + } + function quit() { + quitting = true + pending = null + if (!current) { quitReady(); return } + current.obsolete = true + if (current.running) current.signal(9) + } + function ended(process, reason) { + if (current !== process) return + current = null + process.destroy() + if (quitting) quitReady() + else if (pending) Qt.callLater(launch) + else if (!process.obsolete) failed(reason) + } + Component.onDestruction: if (current && current.running) current.signal(9) + + Component { + id: worker + Process { + id: process + property var request + property bool obsolete: false + property bool started: false + command: [Quickshell.env("FLEA_BIN") || "flea", "--backend"] + stdinEnabled: true + onStarted: { + started = true + if (obsolete || root.quitting) { signal(9); return } + write(JSON.stringify(request) + "\n") + } + stdout: SplitParser { + onRead: function(line) { + if (process.obsolete || root.quitting || root.current !== process || !line) return + try { root.message(JSON.parse(line)) } + catch (error) { root.failed("The listing backend sent an invalid reply.") } + } + } + onExited: function(code, status) { root.ended(process, "The listing backend exited with code " + code) } + // A failed QProcess spawn emits runningChanged, but never exited. + onRunningChanged: if (!running && !started) root.ended(process, "The listing backend could not be started.") + } + } +} diff --git a/ui/picker.qml b/ui/picker.qml index 3d6ea8bc9..3ec85ac95 100644 --- a/ui/picker.qml +++ b/ui/picker.qml @@ -52,7 +52,6 @@ ShellRoot { readonly property int shownTotal: win.total onFilterChanged: if (win.path.length) win.openWithoutHistory(win.path) - // Where Back goes, and it only ever goes back: Parent is its own button and pushes here too. property var history: [] // The save mode's own name, which starts as the caller's suggestion only when that // suggestion is a filename: tools/flea-portal passes current_name through verbatim, so a @@ -99,15 +98,14 @@ ShellRoot { } function open(next) { - if (win.submitting || next === win.path) - return - if (win.path.length > 0) - win.history = win.history.concat([win.path]) + if (win.submitting || next === win.path) return + if (win.path.length > 0) win.history = win.history.concat([win.path]) win.openWithoutHistory(next) } function openWithoutHistory(next) { if (win.backendUnavailable) return + listing.clear() if (shares.item) shares.item.close() win.path = next win.total = 0 @@ -126,8 +124,8 @@ ShellRoot { } function requestListing(request) { - win.pendingListings++ - backend.send(Object.assign(request, win.filterRequest())) + win.pendingListings = 1 + listing.request(Object.assign(request, win.filterRequest())) } function filterRequest() { @@ -161,7 +159,7 @@ ShellRoot { win.saveRequest = win.check({op: "save", folder: win.path, name: win.saveName}) } - // A property var does not notify on an in-place mutation, so history is reassigned, never popped. + // Reassign history so property var notifies. function goBack() { if (shares.item && shares.item.active) { shares.item.close(); return } if (win.submitting || win.history.length === 0) @@ -247,8 +245,7 @@ ShellRoot { win.finish(Picker.RESPONSE_CANCELLED, []) } - // The one write out of this process. The window closes only once the reply file is on disk, - // because tools/flea-portal reads it after this process exits and a lost write is a fault. + // The portal reads this reply after exit, so its atomic write must finish before teardown. function finish(response, list) { if (win.answered) return @@ -280,18 +277,15 @@ ShellRoot { // The reply file does not exist until this window writes it, and a preload read of a // path that is not there is not an error worth a line; onSaveFailed below is. printErrors: false - // Sequenced on saved(), never on setText() returning: the answer has to be readable - // before this process ends, and Quickshell writes it on its own thread. The backend is - // told next, ui/shell.qml's own exit gate, so no listing child outlives this window. - onSaved: backend.quit() + // Only the persisted reply releases the caller; both read-only children are reaped first. + onSaved: lifecycle.quit() onSaveFailed: { console.warn("the portal reply could not be written, so the request fails rather than reporting a refusal") - backend.quit() + lifecycle.quit() } } - // Closing the window is a refusal, the board's own rule, and it takes the same path a - // pressed Cancel does. A window closed after an answer is the answer's own teardown. + // A window-manager close takes the same refusal path as Cancel. Connections { target: Quickshell function onLastWindowClosed() { @@ -301,18 +295,25 @@ ShellRoot { } } - // Quickshell 0.3.1 has no exit API and Qt.quit() is a no-op, so the window signals itself, - // exactly as ui/shell.qml does, once the backend says it has drained. - Connections { target: backend; function onQuitReady() { Quickshell.execDetached(["kill", String(Quickshell.processId)]) } } + // The saved reply can leave once both chooser-owned read processes have exited. + Flea.PickerLifecycle { + id: lifecycle; checks: backend; listing: listing + onStopped: Quickshell.execDetached(["kill", String(Quickshell.processId)]) + } + Flea.PickerListing { + id: listing + onFailed: function(reason) { backend.failed("scan", win.path, reason, 0) } + onMessage: function(message) { backend.receive(JSON.stringify(message)) } + } Flea.Backend { id: backend + pickerOnly: true onListed: function (n, readMs, sortMs) { if (win.backendUnavailable) return - win.pendingListings = Math.max(0, win.pendingListings - 1) - win.receivingLatestListing = win.pendingListings === 0 - if (!win.receivingLatestListing) return + win.pendingListings = 0 + win.receivingLatestListing = true win.total = n win.listingState = n === 0 ? "empty" : "ready" } @@ -339,8 +340,7 @@ ShellRoot { return } if (where === "scan" || where === "sort") { - win.pendingListings = Math.max(0, win.pendingListings - 1) - if (win.pendingListings > 0) return + win.pendingListings = 0 } win.listingState = "empty" win.say(msg, true) @@ -428,7 +428,7 @@ ShellRoot { anchors.top: chrome.bottom anchors.bottom: save.top picker: win - backend: backend + backend: listing clip: true focus: true enabled: !win.submitting && !win.backendUnavailable diff --git a/ui/qmldir b/ui/qmldir index 4241e2ca6..878ecfc56 100644 --- a/ui/qmldir +++ b/ui/qmldir @@ -103,3 +103,5 @@ TrashHost 1.0 TrashHost.qml PaneMenuActions 1.0 PaneMenuActions.qml PaneStates 1.0 PaneStates.qml MenuActionDialog 1.0 MenuActionDialog.qml +PickerListing 1.0 PickerListing.qml +PickerLifecycle 1.0 PickerLifecycle.qml