drm: bound the body read, and stop the empty key from aliasing displays

From the second review bot on our fork. Two of these are real and one of them is
mine from earlier today.

A raw body read had no deadline. Only the header was bounded, and drm_read_full
loops on readable() until it has the exact length, so a producer that wrote a
header and then stopped (crashed, stopped, wedged) pinned the consumer receive
thread forever. That thread is also the one that observes the stop flag, so every
capturer rebuild would have stranded another thread and its render context. The
whole body is bounded now, and an overrun is a hard error because the header is
already consumed and the frame cannot be resumed.

get_capturer_info collapsed an unknown connector identity to the empty string and
then read and wrote the health map under it, so two unidentifiable displays shared
one entry and one could demote the other. That is exactly the aliasing frame()
refuses to take part in; I fixed one side of it this morning and left the other.
The key is an Option now and both blocks skip when it is None: a display with no
identity simply carries no health.

Also from the same pass, smaller:

- build.py validates the shape of DRMTAP_SHA and DRMTAP_REPO before they reach a
  shell command. Both are env-overridable and get interpolated, and beyond the
  injection argument, an abbreviated sha would defeat the point of pinning while
  failing in a much less obvious place.
- the workflow's push path list is now identical to the pull_request one. It was
  missing four paths, so a push to master touching only those would have skipped
  re-verification.
- the checkouts set persist-credentials: false, so the token does not stay in
  .git/config for the rest of the job.
- a concurrency group supersedes a stale PR run, but never cancels a master run,
  whose whole purpose is to record that a commit was verified.

Not taken: reading VCPKG_COMMIT_ID and FLUTTER_VERSION from a shared .env. There
is no .env at the repo root, and the stock ci.yml and flutter-build.yml hardcode
those same two values, so this matches what is already there.

101 tests pass, both configs build.
This commit is contained in:
Mariano Abad
2026-07-28 10:46:56 -03:00
parent a0bb909f4e
commit 6b93f339ba
4 changed files with 56 additions and 5 deletions

View File

@@ -6,6 +6,12 @@ name: DRM capture (opt-in drm feature)
permissions:
contents: read
# Supersede a stale run when a PR is pushed again; never cancel a master run, whose whole job is to
# record that a given commit on master was verified.
concurrency:
group: drm-capture-${{ github.event.pull_request.number || github.ref }}
cancel-in-progress: ${{ github.event_name == 'pull_request' }}
# Everything CI-side about the opt-in `drm` backend lives here, so the stock CI and release workflows
# stay byte-identical to a build with the feature off. Nothing in this file runs unless a drm-related
# path changes (or someone dispatches it by hand), so a PR that does not touch the backend pays nothing.
@@ -32,13 +38,19 @@ on:
push:
branches:
- master
# Deliberately the SAME list as the pull_request trigger above: a shorter one here means a push
# that touches only the missing paths (a squash merge, a direct push) skips re-verification.
paths:
- "libs/scrap/src/common/drm_reader.rs"
- "libs/scrap/src/common/drm_render.rs"
- "libs/scrap/src/common/drmtap_dl.rs"
- "libs/scrap/src/common/mod.rs"
- "libs/scrap/Cargo.toml"
- "src/ipc.rs"
- "src/ipc/**"
- "src/server/drm_capturer.rs"
- "src/server/wayland.rs"
- "src/server/display_service.rs"
- "build.py"
- ".github/workflows/drm-capture.yml"
@@ -66,6 +78,7 @@ jobs:
uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4
with:
submodules: recursive
persist-credentials: false
- name: Install prerequisites
shell: bash
@@ -121,6 +134,8 @@ jobs:
steps:
- name: Checkout source code
uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4
with:
persist-credentials: false
- name: Install libdrmtap build deps
shell: bash
@@ -199,6 +214,7 @@ jobs:
uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4
with:
submodules: recursive
persist-credentials: false
- name: Restore bridge files
uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1