From c6f864ea309c75d4119417a1d7a25ce4bc8f63bb Mon Sep 17 00:00:00 2001 From: sami Date: Sat, 12 Sep 2026 21:27:28 +0400 Subject: [PATCH 1/4] gui: fix RpcClient double-free on stop() and the 1000-row list cap MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both found live while building gui/tests/dod's DoD harness, not from reading the code — see that commit for how. RpcClient::stop() left conn_ dangling after joining the worker thread: the thread's own finish() flushes the DeferredDelete stop()'s connect(&thread_, &QThread::finished, conn_, &QObject::deleteLater) already posted, so conn_ is gone by the time stop() returns, but nothing cleared the pointer. Any caller that calls stop() and later lets the client destruct (the harness's own client.stop() at shutdown; also plain, correct API usage) hit a double-free in the destructor's leftover `delete conn_`. Caught by ASan on the very first run that actually exercised the stop-then-destroy path. requestInitialList() also called download.list with a hardcoded `{"limit": 1000}`, silently capping the table at 1000 rows no matter how many the daemon actually has — download.list.schema.json's own description says "the GUI pages", not "the GUI takes it all in one call". The scroll-60fps DoD gate refused to run against mockd --tasks 10000 rather than "pass" against a 1000-row table, which is what surfaced it. requestInitialList() now pages (5000 per call, the schema's own max) until `total` is satisfied, then resets the model once with everything. Separately: RpcConnection's session.subscribe list never included event.settings.changed or event.grabber.progress, even though RpcClient has carried signals for both since the Options/Grabber work — session.subscribe "replaces the previous selection" and "nothing is delivered until this is called", so both events were being silently dropped by any real daemon that enforces the subscription (mockd does; verified live with a second subscribed client actually receiving event.settings.changed after this fix, round-tripped through a real veloxd's settings.set). GrabberWizard's 5 s poll fallback is exactly why this went unnoticed until now — it covered for the missing push the whole time. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01NSCdCWFXBTSBBK3MzWtJiC --- gui/src/rpc/RpcClient.cpp | 46 ++++++++++++++++++++++++++++++----- gui/src/rpc/RpcClient.hpp | 1 + gui/src/rpc/RpcConnection.cpp | 10 +++++--- 3 files changed, 47 insertions(+), 10 deletions(-) diff --git a/gui/src/rpc/RpcClient.cpp b/gui/src/rpc/RpcClient.cpp index 529950c..c1cdb95 100644 --- a/gui/src/rpc/RpcClient.cpp +++ b/gui/src/rpc/RpcClient.cpp @@ -56,6 +56,13 @@ void RpcClient::stop() { QMetaObject::invokeMethod(conn_, "stop", Qt::QueuedConnection); thread_.quit(); thread_.wait(); + // thread_.wait() does not return until thread_'s own finish() has already flushed the + // DeferredDelete this class's own connect(&thread_, &QThread::finished, conn_, + // &QObject::deleteLater) posted — conn_ is gone by now. Null it out so a later call + // (stop() is a public slot; a caller stopping and then destroying the client is normal + // use, and the destructor's own `delete conn_` for the never-started case must not + // run a second time against memory this path already freed). + conn_ = nullptr; } void RpcClient::call(const QString &methodName, const QJsonObject ¶ms, @@ -76,17 +83,44 @@ void RpcClient::onConnectionState(int state) { } void RpcClient::requestInitialList() { - call(QString::fromLatin1(method::kDownloadList), QJsonObject{{"limit", 1000}}, - [this](const RpcReply &reply) { + fetchListPage(0, {}); +} + +// download.list.schema.json: "Filtering, sorting and paging all happen in the daemon so +// the GUI never materializes 100k rows to show 40" — limit maxes out at 5000, so one call +// cannot ever return everything for a table the DoD's own gate says can hold 10 000 rows. +// A single fixed-limit call here silently truncated the table below that (caught by +// gui/tests/dod's scroll-60fps gate refusing to run against a 1000-row table when mockd +// seeded 10000). Page until `total` is satisfied, then reset the model exactly once. +void RpcClient::fetchListPage(int offset, QJsonArray accumulated) { + constexpr int kPageSize = 5000; // download.list's own maximum + constexpr int kMaxPages = 100; // 500 000 rows — a safety cap, not an expected ceiling + call(QString::fromLatin1(method::kDownloadList), + QJsonObject{{"offset", offset}, {"limit", kPageSize}}, + [this, offset, accumulated](const RpcReply &reply) mutable { if (!reply.ok()) { qCWarning(lcRpc, "download.list failed: %d %s", reply.error.code, qUtf8Printable(reply.error.message)); + if (!accumulated.isEmpty()) { + emit taskListReset(accumulated); // show what we got rather than nothing + } return; } - const QJsonArray items = reply.result.toObject().value("items").toArray(); - qCInfo(lcRpc, "initial download.list: %lld row(s)", - static_cast(items.size())); - emit taskListReset(items); + const QJsonObject result = reply.result.toObject(); + const QJsonArray page = result.value("items").toArray(); + const qint64 total = static_cast(result.value("total").toDouble()); + for (const QJsonValue &item : page) { + accumulated.append(item); + } + const bool morePages = + !page.isEmpty() && accumulated.size() < total && (offset / kPageSize) < kMaxPages; + if (morePages) { + fetchListPage(offset + static_cast(page.size()), accumulated); + return; + } + qCInfo(lcRpc, "initial download.list: %lld of %lld row(s)", + static_cast(accumulated.size()), static_cast(total)); + emit taskListReset(accumulated); }); } diff --git a/gui/src/rpc/RpcClient.hpp b/gui/src/rpc/RpcClient.hpp index 80dec01..ba1270f 100644 --- a/gui/src/rpc/RpcClient.hpp +++ b/gui/src/rpc/RpcClient.hpp @@ -67,6 +67,7 @@ class RpcClient : public QObject { private: void requestInitialList(); + void fetchListPage(int offset, QJsonArray accumulated); QThread thread_; RpcConnection *conn_ = nullptr; // owned by thread_ affinity, deleted on thread finish diff --git a/gui/src/rpc/RpcConnection.cpp b/gui/src/rpc/RpcConnection.cpp index ced660c..e27c69c 100644 --- a/gui/src/rpc/RpcConnection.cpp +++ b/gui/src/rpc/RpcConnection.cpp @@ -154,10 +154,12 @@ void RpcConnection::dispatchFrame(const QJsonObject &frame) { socket_->abort(); // version mismatch or refused — bounce and retry return; } - sendRaw(kSubscribeId, QString::fromLatin1(method::kSessionSubscribe), - QJsonObject{{"events", QJsonArray{event::kTaskAdded, event::kTaskRemoved, - event::kTaskState, event::kTaskProgress, - event::kSpeedGlobal, event::kNotify}}}); + sendRaw( + kSubscribeId, QString::fromLatin1(method::kSessionSubscribe), + QJsonObject{ + {"events", QJsonArray{event::kTaskAdded, event::kTaskRemoved, event::kTaskState, + event::kTaskProgress, event::kSpeedGlobal, event::kNotify, + event::kSettingsChanged, event::kGrabberProgress}}}); return; } if (id == kSubscribeId) { From 755d85964eb3dcc8ceffee624382d9db8735f07e Mon Sep 17 00:00:00 2001 From: sami Date: Sat, 12 Sep 2026 21:28:36 +0400 Subject: [PATCH 2/4] =?UTF-8?q?gui:=20M1=20DoD=20harness=20=E2=80=94=20scr?= =?UTF-8?q?oll-60fps,=20rss-flat,=20unhappy-path?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit gui/docs/pkg-qa-requests-m1.md R3, filed by the previous session: three GUI M1 DoD items (10k rows at 60 fps, flat RSS over 10 minutes, --slow/--flaky/--drop-connection recovery) had nowhere to run in CI. This is the harness — `gui/tests/dod/run.sh [--json ]`, exactly the path/invocation contract tests/integration/README.md already specified — plus `gui/tests/dod/dod_harness.cpp`, the Qt/RpcClient-driven binary that actually runs each gate against a real mockd run.sh starts and tears down itself. - scroll-60fps: an eased scripted scroll over the whole loaded table, timing each step's synchronous repaint; p99 against a 16.6 ms budget (auto-scaled 4x under a sanitized build — ASan/UBSan overhead, not a loosened bar, see the harness's isSanitizedBuild()). - rss-flat: samples this process's own VmRSS at 1 Hz across the run, discards a warm-up window, checks post-warm-up growth against a stated 20 MiB slack. - unhappy-path: three phases (slow/flaky/drop-connection), each its own mockd instance; passes when the client reaches and holds Connected with no crash or hang. A watchdog (the harness's own QTimer, backstopped by run.sh's external `timeout`) turns a genuine hang into a bounded non-zero exit rather than needing the CI caller to timeout(1) around it. Every gate honours the exit-code and --json contract PKG/QA's pre-drafted CI job expects unchanged (one addition needed: the build step must also build the `gui-dod-harness` target, noted in the R3 update). No leaked mockd processes on any exit path (`trap cleanup EXIT INT TERM`); no writes outside a tempdir except the caller's own --json path. Verified live end-to-end (not just unit-level): all three gates run against a real mockd under the exact `ASAN_OPTIONS=detect_leaks=1:halt_on_error=1` `.github/workflows/ci.yml`'s sanitizers job already sets, all pass, and scroll-60fps was forced red once on purpose (VELOX_DOD_FRAME_BUDGET_MS=1) to prove the fail path and exit code actually work. Building this is also what surfaced the two RpcClient bugs fixed in the previous commit, and one real gap in mockd itself — --drop-connection never worked over the Unix socket transport (only WebSocket) — filed as gui/docs/proto-requests-m1.md since tools/mockd is PROTO's file. gui/docs/pkg-qa-requests-m1.md R3 and R4 (an unrelated, non-blocking Qt6::DBus CMake hygiene note filed while wiring the clipboard global-shortcut path) are updated with the concrete findings above. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01NSCdCWFXBTSBBK3MzWtJiC --- gui/docs/pkg-qa-requests-m1.md | 113 +++++++-- gui/docs/proto-requests-m1.md | 105 +++++++++ gui/tests/CMakeLists.txt | 3 + gui/tests/dod/CMakeLists.txt | 11 + gui/tests/dod/dod_harness.cpp | 416 +++++++++++++++++++++++++++++++++ gui/tests/dod/run.sh | 197 ++++++++++++++++ 6 files changed, 826 insertions(+), 19 deletions(-) create mode 100644 gui/docs/proto-requests-m1.md create mode 100644 gui/tests/dod/CMakeLists.txt create mode 100644 gui/tests/dod/dod_harness.cpp create mode 100755 gui/tests/dod/run.sh diff --git a/gui/docs/pkg-qa-requests-m1.md b/gui/docs/pkg-qa-requests-m1.md index 96579be..f4acf3b 100644 --- a/gui/docs/pkg-qa-requests-m1.md +++ b/gui/docs/pkg-qa-requests-m1.md @@ -121,26 +121,101 @@ Notes for whoever applies it: --- -## R3 — the GUI DoD gates have nowhere to run in CI +## R3 — the GUI DoD gates have nowhere to run in CI — RESOLVED, ready to wire in -To be precise about what already works: `VELOX_BUILD_GUI` defaults `ON`, the `ci` preset -inherits `dev`, and once `gui/` is on `main` the `build` and `sanitizers` jobs configure -and build `velox-gui` and run `ctest --preset ci`, which picks up all three GUI checks -(`gui_downloadtablemodel`, `gui_rtl`, `gui_no_download_logic`). That part is covered. +**Status: done on GUI's side.** The harness `tests/integration/README.md` was waiting on +now exists, builds, and has been run end-to-end (all three gates, all three unhappy-path +sub-phases) against a real `mockd` with no changes needed to the pre-drafted job below. -What has no home is the part of the GUI M1 definition of done that isn't a unit test: +**Path:** `gui/tests/dod/run.sh [--json ]`, exactly the contract +`tests/integration/README.md` specifies. `` is `scroll-60fps` | `rss-flat` | +`unhappy-path`. It builds and tears down its own `mockd` (isolated `XDG_RUNTIME_DIR` via +`mktemp -d`), needs no network, and leaves nothing running on any exit path (`trap +cleanup EXIT INT TERM`) — verified live by checking for orphaned `tsx`/`mockd` processes +after both a passing and a forced-failing run of each gate. -1. **10 000 rows scroll at 60 fps** (`mockd --tasks 10000`) — needs a frame-timing probe - against the offscreen (or Xvfb) view; red when a scroll frame exceeds ~16 ms at the - 99th percentile. -2. **Flat memory over 10 minutes of progress events** — needs RSS sampled across a - 10-minute `mockd --tasks 10000` run; red when RSS grows more than a small fixed slack - (a leak in the progress-patch path is the thing this catches). -3. **Unhappy-path recovery** — `mockd --slow`, `--flaky `, `--drop-connection `: - the client must show the banner and recover without a freeze or crash; red on a crash, - a hang (watchdog), or the connection state never returning to `Connected`. +**One change from the pre-drafted job:** the "Configure + build" step needs to also build +the harness binary, not just `velox-gui`: +```diff +- cmake --build --preset dev --target velox-gui ++ cmake --build --preset dev --target velox-gui gui-dod-harness +``` +Nothing else in the pre-drafted YAML needs to change — the `# TODO(GUI): path` comment on +the `scroll-60fps` line can just come off along with the marker on the line below it. -GUI owns writing that harness (`gui/tests/` + a driver script, headless against `mockd`). -Wiring it into `.github/workflows/ci.yml` as its own job — with the 10-minute one likely -`nightly` rather than per-PR — is PKG/QA. Say the word and it comes over as a follow-up -request with the job stanza pre-written. +**`rss-flat`'s slack: 20 MiB (`VELOX_DOD_RSS_SLACK_KIB`, default `20480`).** Chosen by +running the gate locally (`VELOX_DOD_RSS_DURATION_SEC=8` override, i.e. not the real +10-minute number) a handful of times against `mockd --tasks 10000 --seed 1` and looking at +actual post-warm-up growth (single-digit MiB per run here) — 20 MiB gives real headroom +above that noise floor without being so loose a genuine per-tick leak in the progress-patch +path could hide under it. This has **not** been proven against a full real 10-minute +sanitized run (that's the nightly job's own first execution) or tuned against +production-length data yet; treat it as GUI's stated starting number, not a +load-tested constant, and expect it may need retuning after the first few real +`gui-dod-nightly` runs land actual series data. + +**One caveat worth deciding on explicitly: the pre-drafted job builds with `cmake --preset +dev`, i.e. ASan+UBSan (CLAUDE.md: "default for all lanes").** `scroll-60fps`'s frame +budget already compensates (`gui/tests/dod/dod_harness.cpp` multiplies 16.6 ms by 4× when +it detects a sanitized build — measured p99 here was ~50 ms against ASan overhead, well +under the scaled 66.4 ms budget, so the multiplier is doing real work, not padding for no +reason). `rss-flat`'s slack does **not** get a similar adjustment — ASan's allocator +(redzones, quarantine) can look like real growth over a long run in a way this hasn't been +validated against yet. Two honest options: run `gui-dod-nightly` against a `release`-preset +build instead of `dev` (loses the sanitizers' own bug-catching value for this one job), or +accept `VELOX_DOD_RSS_SLACK_KIB` may need a second, larger number for the ASan build once +real 10-minute data exists. GUI's preference is the second (keep sanitizers on +everywhere), but this is genuinely PKG/QA's call since it's their job definition. + +**Verified live**, under the exact `ASAN_OPTIONS=detect_leaks=1:halt_on_error=1` the +`sanitizers` job already sets (checked against `.github/workflows/ci.yml` rather than +assumed) — no leak-suppression flag needed, on any of the three gates: + +* `scroll-60fps` against `mockd --tasks 10000 --seed 1`: PASS at p99 ≈ 50 ms (budget + 66.4 ms sanitized). Forced red once via `VELOX_DOD_FRAME_BUDGET_MS=1` to confirm the + fail path and exit code actually work, not just the pass path. +* `rss-flat` at `VELOX_DOD_RSS_DURATION_SEC=6/8`: PASS, ~4-6 MiB growth against the + 20 MiB slack. +* `unhappy-path`, all three phases (`--slow 900`, `--flaky 0.3`, `--drop-connection 5`): + PASS. One real finding from building this: `--drop-connection` is currently a no-op + over the Unix socket transport in `mockd` itself (only wired for WebSocket) — filed as + `gui/docs/proto-requests-m1.md` since that's PROTO's file to fix, not GUI's. The gate + still passes today on the weaker (but real, and the actually-documented) condition that + the client reaches and holds `Connected`; it just isn't proving a real mid-session drop + yet for that one phase. + +Two bugs surfaced and fixed *by* building this harness, both in `gui/`'s own RPC client +(caught by ASan, not assumed): `RpcClient::stop()` left `conn_` dangling after joining its +worker thread, so any caller that called `stop()` and then let the client destruct hit a +double-free — the harness's own `client.stop()` at shutdown found it on the first run. +Separately, `RpcClient`'s initial `download.list` call had a hardcoded `limit: 1000` with +no paging, silently capping the table at 1000 rows regardless of how many the daemon +actually has — `scroll-60fps` against `--tasks 10000` refused to run rather than +"passing" against a 1000-row table, which is what caught it. Both fixed on `lane/gui` +before this request was filed. + +--- + +## R4 — root `CMakeLists.txt`'s `find_package(Qt6 ...)` should list `DBus` explicitly + +Not currently broken — flagging a "works, but by accident" for the record. `gui/src/ +clipboard/GlobalShortcut.cpp` (docs/06-risks-and-spikes.md R2's explicit path #2: +`org.freedesktop.portal.GlobalShortcuts`) needs `Qt6::DBus`. **Verified live: the target +already exists and links today**, even though the root `find_package` doesn't list `DBus` +in `COMPONENTS` — this Qt 6 packaging apparently exports every module's CMake target once +any component pulls in the shared prefix, `DBus` included. `gui/CMakeLists.txt` still +guards the clipboard sources on `if(TARGET Qt6::DBus)` (same pattern the file already uses +for `veloxproto`), so if that turns out to be environment-specific rather than a general +Qt 6 CMake guarantee, the build degrades to "feature skipped," not "build broken," on +whatever machine finds out otherwise. + +Worth making explicit anyway, since relying on undocumented target leakage is fragile: + +```diff +- find_package(Qt6 6.6 REQUIRED COMPONENTS Widgets Svg Network LinguistTools) ++ find_package(Qt6 6.6 REQUIRED COMPONENTS Widgets Svg Network DBus LinguistTools) +``` + +No apt change needed either way — `qt6-base-dev` (already in `APT_GUI`) ships `QtDBus`'s +headers directly (verified live: `dpkg -L qt6-base-dev | grep -i dbus` lists the whole +`QtDBus/` include tree). diff --git a/gui/docs/proto-requests-m1.md b/gui/docs/proto-requests-m1.md new file mode 100644 index 0000000..32439ac --- /dev/null +++ b/gui/docs/proto-requests-m1.md @@ -0,0 +1,105 @@ +# GUI → PROTO requests (M1) + +Filed by lane GUI while building `gui/tests/dod/` (gui/docs/pkg-qa-requests-m1.md R3's +harness). Touches `tools/mockd/` — PROTO-owned (CLAUDE.md §1) — so GUI is not making the +edit. Apply-ready below. + +--- + +## `mockd --drop-connection` is a no-op over the Unix socket transport + +`--drop-connection ` is documented as "terminate every connection every N seconds, to +exercise reconnect logic" and is exactly what `gui/tests/dod/run.sh unhappy-path` needs +for its drop-connection phase. It works — but only over WebSocket. + +**Repro:** `tools/mockd/src/index.ts`'s `startUds()` call passes `args.slow` and stops +there: + +```ts +startUds(args.uds, dispatcher, connections, log, args.slow); +``` + +`startWs()`, two lines below, gets the full options object including `dropEverySec`. +`startUds()`'s own signature (`tools/mockd/src/transport/uds.ts`) has no +`dropEverySec` parameter at all, and nothing in it ever calls `socket.destroy()` — the +periodic-drop `setInterval` that `startWs` has (its last ~6 lines) simply does not exist +on the UDS side. + +**Verified live**, not inferred from reading: ran `mockd --no-ws --drop-connection 5`, +connected `gui/tests/dod/dod_harness unhappy-path --phase drop-connection` against it +(UDS, the GUI's only transport) with a 45 s observation window, and `stateChanged` never +fired — the connection sat in `Connected` the entire time. Same command with `--flaky 0.3` +correctly leaves the connection state alone (that flag only fails individual call +replies, which is right), so this is specific to `--drop-connection` and the UDS +transport, not a harness-side detection problem. + +**Effect:** every GUI/CLI/nmhost consumer of mockd — the only transport they actually +use — cannot be tested against a dropped connection at all today. `gui/tests/dod/run.sh` +ships its `unhappy-path` drop-connection phase anyway (log intentionally records +`sawDisruption` in its JSON so this is visible, not silently green), but it is currently +only proving the client survives 45 quiet seconds, not a real drop. + +### Fix — mirror `ws.ts`'s existing pattern onto `uds.ts` + +**`tools/mockd/src/transport/uds.ts`:** +```diff + export function startUds( + path: string, + dispatcher: Dispatcher, + connections: Set, + log: (msg: string) => void, + delayMs: number, ++ dropEverySec: number = 0, + ): Server { + mkdirSync(dirname(path), { recursive: true }); + rmSync(path, { force: true }); + ++ const sockets = new Set(); + const server = createServer((socket: Socket) => { ++ sockets.add(socket); + const session: Session = { transport: 'uds', paired: true, subscribed: new Set(), sessionId: randomUUID() }; + const conn: Connection = { + session, + send: (frame) => { + if (!socket.destroyed) socket.write(JSON.stringify(frame) + '\n'); + }, + }; + connections.add(conn); + log(`uds: client connected (${connections.size} open)`); + ... + socket.on('error', (err) => log(`uds: socket error: ${err.message}`)); + socket.on('close', () => { + connections.delete(conn); ++ sockets.delete(socket); + log(`uds: client disconnected (${connections.size} open)`); + }); + }); + + server.listen(path, () => log(`uds: listening on ${path}`)); ++ ++ if (dropEverySec > 0) { ++ setInterval(() => { ++ log(`uds: dropping ${sockets.size} connection(s) (--drop-connection)`); ++ for (const s of sockets) s.destroy(); ++ }, dropEverySec * 1000).unref(); ++ } ++ + return server; + } +``` + +**`tools/mockd/src/index.ts`** (~line 205): +```diff +- startUds(args.uds, dispatcher, connections, log, args.slow); ++ startUds(args.uds, dispatcher, connections, log, args.slow, args.dropEverySec); +``` + +Both use `.unref()`/existing shutdown handling already in `index.ts`, so no change needed +there. `socket.destroy()` (vs. `.end()`) matches `ws.ts`'s `.terminate()` — an abrupt drop, +which is the point of the flag. + +Not urgent for M0/M1 GUI work — `gui/tests/dod/run.sh`'s other two unhappy-path phases +(`--slow`, `--flaky`) both work correctly over UDS today, and the drop-connection phase +still exercises 45 s of otherwise-idle connection handling. But the flag's whole purpose +is unmet on the transport every real consumer uses, and the fix is a direct port of code +that already exists two files over. diff --git a/gui/tests/CMakeLists.txt b/gui/tests/CMakeLists.txt index 01d36a6..221ce58 100644 --- a/gui/tests/CMakeLists.txt +++ b/gui/tests/CMakeLists.txt @@ -144,6 +144,9 @@ set_tests_properties(gui_grabberwizard PROPERTIES LABELS "gui" ENVIRONMENT "QT_QPA_PLATFORM=offscreen") +# gui-dod-harness — the M1 DoD gates (scroll-60fps / rss-flat / unhappy-path). +add_subdirectory(dod) + # gui_no_download_logic — CLAUDE.md §3 as an executable check, not a hope. # Red when: a download-logic token (curl, raw pwrite, sqlite, QSqlDatabase) appears # under gui/src. grep exits 0 only when it finds a match, so a hit fails the test. diff --git a/gui/tests/dod/CMakeLists.txt b/gui/tests/dod/CMakeLists.txt new file mode 100644 index 0000000..1413114 --- /dev/null +++ b/gui/tests/dod/CMakeLists.txt @@ -0,0 +1,11 @@ +# gui-dod-harness — the GUI M1 DoD gates. Lane GUI. +# +# Not a ctest target: run.sh invokes this directly against a mockd it starts and +# tears down itself (gui/docs/pkg-qa-requests-m1.md R3). Built under the same +# VELOX_BUILD_TESTS gate as the rest of gui/tests since it only ever runs in CI/dev, never +# ships. + +add_executable(gui-dod-harness dod_harness.cpp) +target_compile_features(gui-dod-harness PRIVATE cxx_std_23) +target_compile_options(gui-dod-harness PRIVATE -Wall -Wextra -Wpedantic -Werror) +target_link_libraries(gui-dod-harness PRIVATE velox-gui-lib Qt6::Widgets) diff --git a/gui/tests/dod/dod_harness.cpp b/gui/tests/dod/dod_harness.cpp new file mode 100644 index 0000000..45e0c61 --- /dev/null +++ b/gui/tests/dod/dod_harness.cpp @@ -0,0 +1,416 @@ +// GUI M1 DoD gate harness. Lane GUI. +// +// gui/docs/pkg-qa-requests-m1.md R3 / tests/integration/README.md: PKG/QA's CI job +// invokes this (via run.sh) as ` --sock [--json ]`, one gate per run: +// +// scroll-60fps — mockd --tasks 10000, a scripted scroll over the whole table; fail on +// a p99 per-step paint time over budget (16.6 ms, i.e. 60 fps). +// rss-flat — mockd --tasks 10000 streaming progress for --duration-sec (default +// 600 = 10 min); fail if RSS grows past a fixed slack after warm-up. +// unhappy-path — one phase (--phase slow|flaky|drop-connection, label only: the actual +// mockd flag is run.sh's job) against a client that must reach +// Connected and hold it, no crash, no hang. +// +// Exit 0 pass, non-zero fail. --json writes one result object. A watchdog timer +// converts a hang into a non-zero exit itself — nothing here should ever need an external +// timeout(1) to end it. +// +// "Fling scroll" and "frame" are approximate in a headless/offscreen run: there is no +// compositor to hand a real frame to, so what is measured is wall-clock time for one +// scroll step's model-driven repaint — the CPU cost a real frame would also have to pay, +// just without a GPU present/vsync on top of it. That is the part a progress-patch +// regression or a delegate doing needless work would actually blow. + +#include +#include +#include +#include +#include +#include + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "models/DownloadTableModel.hpp" +#include "rpc/RpcClient.hpp" +#include "widgets/ProgressDelegate.hpp" + +using velox::gui::DownloadTableModel; +using velox::gui::ProgressDelegate; +namespace rpc = velox::gui::rpc; + +namespace { + +// Pumps the event loop in small slices until `pred` is true or `timeoutMs` elapses. +// Never blocks longer than that — every wait in this file is bounded, which is what lets +// the process reach its own exit(1) instead of needing the watchdog for the common case. +bool waitFor(const std::function &pred, int timeoutMs) { + QElapsedTimer t; + t.start(); + while (!pred() && t.elapsed() < timeoutMs) { + QCoreApplication::processEvents(QEventLoop::AllEvents, 20); + } + return pred(); +} + +qint64 readRssKiB() { + QFile f(QStringLiteral("/proc/self/status")); + if (!f.open(QIODevice::ReadOnly | QIODevice::Text)) { + return -1; + } + static const QRegularExpression kWs(QStringLiteral("\\s+")); + for (const QByteArray &lineBytes : f.readAll().split('\n')) { + const QString line = QString::fromLatin1(lineBytes); + if (line.startsWith(QLatin1String("VmRSS:"))) { + const QStringList parts = line.split(kWs, Qt::SkipEmptyParts); + if (parts.size() >= 2) { + bool ok = false; + const qint64 kib = parts[1].toLongLong(&ok); + return ok ? kib : -1; + } + } + } + return -1; +} + +double percentile(std::vector v, double p) { + if (v.empty()) { + return 0.0; + } + std::sort(v.begin(), v.end()); + int idx = static_cast(std::ceil(p * static_cast(v.size()))) - 1; + idx = std::clamp(idx, 0, static_cast(v.size()) - 1); + return v[static_cast(idx)]; +} + +// ASan/UBSan add real overhead to every paint; the pre-drafted CI job builds this with +// `cmake --preset dev`, which is ASan+UBSan by default (CLAUDE.md: "default for all +// lanes"). Scaling the budget under a sanitized build is an honest adjustment for +// instrumentation cost, not a loosened bar — VELOX_DOD_FRAME_BUDGET_MS still overrides it +// outright for whoever wants to tune this per-runner. +bool isSanitizedBuild() { +#if defined(__SANITIZE_ADDRESS__) || defined(__SANITIZE_THREAD__) + return true; +#elif defined(__has_feature) +#if __has_feature(address_sanitizer) || __has_feature(thread_sanitizer) + return true; +#else + return false; +#endif +#else + return false; +#endif +} + +void writeJson(const QString &path, const QJsonObject &obj) { + if (path.isEmpty()) { + return; + } + QFile f(path); + if (!f.open(QIODevice::WriteOnly | QIODevice::Truncate | QIODevice::Text)) { + std::fprintf(stderr, "warning: could not write --json output to %s\n", + qUtf8Printable(path)); + return; + } + f.write(QJsonDocument(obj).toJson(QJsonDocument::Indented)); +} + +void wireModel(rpc::RpcClient *client, DownloadTableModel *model) { + QObject::connect(client, &rpc::RpcClient::taskListReset, model, + &DownloadTableModel::resetFromJson); + QObject::connect(client, &rpc::RpcClient::taskProgress, model, + &DownloadTableModel::applyProgress); + QObject::connect(client, &rpc::RpcClient::taskAdded, model, + &DownloadTableModel::applyTaskAdded); + QObject::connect(client, &rpc::RpcClient::taskStateChanged, model, + &DownloadTableModel::applyTaskState); + QObject::connect(client, &rpc::RpcClient::taskRemoved, model, + &DownloadTableModel::applyTaskRemoved); +} + +int runScroll60Fps(rpc::RpcClient &client, const QString &jsonPath) { + DownloadTableModel model; + wireModel(&client, &model); + + if (!waitFor([&] { return client.state() == rpc::ConnectionState::Connected; }, 15000)) { + std::fprintf(stderr, "FAIL: never reached Connected\n"); + return 1; + } + if (!waitFor([&] { return model.rowCount() >= 9000; }, 15000)) { + std::fprintf(stderr, + "FAIL: table never loaded (rowCount=%d) — run mockd with " + "--tasks 10000\n", + model.rowCount()); + return 1; + } + + QTreeView view; + view.setModel(&model); + view.setUniformRowHeights(true); + view.setItemDelegateForColumn(DownloadTableModel::ColStatus, new ProgressDelegate(&view)); + view.resize(1000, 700); + view.show(); + waitFor([] { return false; }, 100); // let the initial show/layout settle + + auto *bar = view.verticalScrollBar(); + const int maxV = bar->maximum(); + if (maxV <= 0) { + std::fprintf(stderr, "FAIL: nothing to scroll (scrollbar max=%d)\n", maxV); + return 1; + } + + // A scripted "fling": ease-out steps (big jumps first, settling to small ones), the + // shape a real flick-scroll decelerates through, rather than a uniform crawl. + constexpr int kSteps = 240; + std::vector frameMs; + frameMs.reserve(kSteps); + for (int i = 1; i <= kSteps; ++i) { + const double t = static_cast(i) / kSteps; + const double eased = 1.0 - std::pow(1.0 - t, 3.0); + const int value = static_cast(static_cast(maxV) * eased); + + QElapsedTimer frame; + frame.start(); + bar->setValue(value); + QCoreApplication::sendPostedEvents(); + view.viewport()->repaint(); // synchronous: times the paint, not just the request + frameMs.push_back(static_cast(frame.nsecsElapsed()) / 1e6); + } + + const double p99 = percentile(frameMs, 0.99); + const double maxMs = *std::max_element(frameMs.begin(), frameMs.end()); + const double meanMs = + std::accumulate(frameMs.begin(), frameMs.end(), 0.0) / static_cast(frameMs.size()); + + double budgetMs = 16.6; + if (isSanitizedBuild()) { + budgetMs *= 4.0; // instrumentation overhead, not a lowered bar — see isSanitizedBuild() + } + const QString override = qEnvironmentVariable("VELOX_DOD_FRAME_BUDGET_MS"); + if (!override.isEmpty()) { + bool ok = false; + const double v = override.toDouble(&ok); + if (ok) { + budgetMs = v; + } + } + + const bool pass = p99 <= budgetMs; + std::printf("%s: p99=%.2f ms mean=%.2f ms max=%.2f ms budget=%.2f ms over %d steps, %d rows\n", + pass ? "PASS" : "FAIL", p99, meanMs, maxMs, budgetMs, kSteps, model.rowCount()); + + writeJson(jsonPath, QJsonObject{ + {"gate", "scroll-60fps"}, + {"rows", model.rowCount()}, + {"steps", kSteps}, + {"p99Ms", p99}, + {"meanMs", meanMs}, + {"maxMs", maxMs}, + {"budgetMs", budgetMs}, + {"sanitized", isSanitizedBuild()}, + {"pass", pass}, + }); + return pass ? 0 : 1; +} + +int runRssFlat(rpc::RpcClient &client, const QString &jsonPath, int durationSec) { + DownloadTableModel model; + wireModel(&client, &model); + + if (!waitFor([&] { return client.state() == rpc::ConnectionState::Connected; }, 15000)) { + std::fprintf(stderr, "FAIL: never reached Connected\n"); + return 1; + } + if (!waitFor([&] { return model.rowCount() >= 9000; }, 15000)) { + std::fprintf(stderr, + "FAIL: table never loaded (rowCount=%d) — run mockd with " + "--tasks 10000\n", + model.rowCount()); + return 1; + } + + // Kept visible: a hidden model-only run would miss any leak that lives in painting + // (delegate scratch state, style caches) rather than in the model's own row patches. + QTreeView view; + view.setModel(&model); + view.setUniformRowHeights(true); + view.setItemDelegateForColumn(DownloadTableModel::ColStatus, new ProgressDelegate(&view)); + view.resize(1000, 700); + view.show(); + + const int warmupSec = std::min(30, std::max(1, durationSec / 10)); + QJsonArray series; + std::vector afterWarmup; + QEventLoop loop; + QTimer sampler; + sampler.setInterval(1000); + int elapsedSec = 0; + QObject::connect(&sampler, &QTimer::timeout, [&] { + ++elapsedSec; + const qint64 rssKiB = readRssKiB(); + series.append(QJsonObject{{"t", elapsedSec}, {"rssKiB", rssKiB}}); + if (elapsedSec > warmupSec) { + afterWarmup.push_back(rssKiB); + } + if (elapsedSec >= durationSec) { + loop.quit(); + } + }); + sampler.start(); + loop.exec(); + + qint64 growthKiB = 0; + if (afterWarmup.size() >= 2) { + const qint64 minRss = *std::min_element(afterWarmup.begin(), afterWarmup.end()); + growthKiB = afterWarmup.back() - minRss; + } + + qint64 slackKiB = 20 * 1024; // 20 MiB: see gui/docs/pkg-qa-requests-m1.md R3 for why + const QString override = qEnvironmentVariable("VELOX_DOD_RSS_SLACK_KIB"); + if (!override.isEmpty()) { + bool ok = false; + const qint64 v = override.toLongLong(&ok); + if (ok) { + slackKiB = v; + } + } + + const bool pass = afterWarmup.size() >= 2 && growthKiB <= slackKiB; + std::printf("%s: growth=%lld KiB slack=%lld KiB over %ds (warmup %ds), %d rows\n", + pass ? "PASS" : "FAIL", static_cast(growthKiB), + static_cast(slackKiB), durationSec, warmupSec, model.rowCount()); + + writeJson(jsonPath, QJsonObject{ + {"gate", "rss-flat"}, + {"rows", model.rowCount()}, + {"durationSec", durationSec}, + {"warmupSec", warmupSec}, + {"growthKiB", growthKiB}, + {"slackKiB", slackKiB}, + {"series", series}, + {"pass", pass}, + }); + return pass ? 0 : 1; +} + +int runUnhappyPath(rpc::RpcClient &client, const QString &jsonPath, const QString &phase) { + bool sawDisconnectOrReconnecting = false; + QObject::connect(&client, &rpc::RpcClient::stateChanged, &client, [&](rpc::ConnectionState s) { + if (s == rpc::ConnectionState::Reconnecting || s == rpc::ConnectionState::Disconnected) { + sawDisconnectOrReconnecting = true; + } + }); + + // 45 s covers mockd's slowest documented --slow value plus a couple of backoff + // cycles; the harness's own watchdog (see main()) is the real ceiling on a hang. + constexpr int kObserveMs = 45000; + const bool reachedConnected = + waitFor([&] { return client.state() == rpc::ConnectionState::Connected; }, kObserveMs); + + QElapsedTimer t; + t.start(); + while (t.elapsed() < kObserveMs) { + QCoreApplication::processEvents(QEventLoop::AllEvents, 50); + } + + const bool finalConnected = client.state() == rpc::ConnectionState::Connected; + const bool pass = reachedConnected && finalConnected; + std::printf("%s [%s]: reachedConnected=%d finalConnected=%d sawDisruption=%d\n", + pass ? "PASS" : "FAIL", qUtf8Printable(phase), reachedConnected, finalConnected, + sawDisconnectOrReconnecting); + + writeJson(jsonPath, QJsonObject{ + {"gate", "unhappy-path"}, + {"phase", phase}, + {"reachedConnected", reachedConnected}, + {"finalConnected", finalConnected}, + {"sawDisruption", sawDisconnectOrReconnecting}, + {"pass", pass}, + }); + return pass ? 0 : 1; +} + +} // namespace + +int main(int argc, char **argv) { + QApplication app(argc, argv); + qRegisterMetaType(); + qRegisterMetaType(); + + QCommandLineParser parser; + parser.setApplicationDescription( + QStringLiteral("GUI M1 DoD gate harness (gui/docs/pkg-qa-requests-m1.md R3)")); + parser.addHelpOption(); + parser.addPositionalArgument(QStringLiteral("gate"), + QStringLiteral("scroll-60fps | rss-flat | unhappy-path")); + QCommandLineOption sockOpt(QStringLiteral("sock"), QStringLiteral("veloxd UDS socket path"), + QStringLiteral("path")); + QCommandLineOption jsonOpt(QStringLiteral("json"), + QStringLiteral("write one result object here"), + QStringLiteral("path")); + QCommandLineOption durationOpt(QStringLiteral("duration-sec"), + QStringLiteral("rss-flat duration override (default 600)"), + QStringLiteral("sec")); + QCommandLineOption phaseOpt(QStringLiteral("phase"), + QStringLiteral("unhappy-path sub-phase label, for the JSON only"), + QStringLiteral("phase"), QStringLiteral("unspecified")); + parser.addOption(sockOpt); + parser.addOption(jsonOpt); + parser.addOption(durationOpt); + parser.addOption(phaseOpt); + parser.process(app); + + const QStringList pos = parser.positionalArguments(); + if (pos.isEmpty() || !parser.isSet(sockOpt)) { + std::fprintf(stderr, + "usage: dod_harness --sock [--json ] " + "[--duration-sec ] [--phase