detached 3592453

test(screencast): saturate pending table without blocking owner

Drive admission and caller completion deterministically with real private-bus replies. Blocking resolver Open on the owner could prevent the overflow request from being processed. Remove the blocking hook and bound reply reads by closing the client on timeout.
commit
3592453f463d671ba6231196bb16ec235441c911
author
brice · · 2026-10-19 18:41 UTC
parent
7edbc98
signature
GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email
checks
1/1
2 files · +35 -25
M internal/adapters/screencast/bus_test.go +31 -13
@@ -374,26 +374,44 @@ func TestNonOwnerIsDenied(t *testing.T) {
374374 // full, the next one gets LimitsExceeded, and the parked ones are answered
375375 // once the check ends.
376376 func TestPendingTableFull(t *testing.T) {
377- h := newSvcHarness(t, harnessOpts{holdOp: true})
378- c := h.client()
379- h.waitName(c, nameIntrospect)
380- serials := make([]uint32, 0, maxPending+1)
381- for range maxPending + 1 {
382- c.NewCall(nameIntrospect, pathIntrospect, ifaceIntrospect, "GetWindows", "")
383- serial, err := c.Send()
384- if err != nil {
385- t.Fatal(err)
386- }
387- serials = append(serials, serial)
377+ // Drive admission on this goroutine, delaying the caller-check transition
378+ // rather than blocking Open on the service owner. A PID reply may arrive
379+ // before all calls do; blocking Open then prevents the owner filling the
380+ // table or answering the overflow at all.
381+ addr, _ := startBus(t)
382+ c, err := zerobus.Dial(addr)
383+ if err != nil {
384+ t.Fatal(err)
388385 }
386+ t.Cleanup(func() { _ = c.Close() })
387+ server, err := zerobus.Dial(addr)
388+ if err != nil {
389+ t.Fatal(err)
390+ }
391+ t.Cleanup(func() { _ = server.Close() })
392+ s := New(captureallow.Builtin, nil, testLog(&logBuf{}))
393+ s.conn = server
394+ peer := &s.callers[0]
395+ *peer = caller{state: callerPid, name: c.UniqueName()}
396+ for i := range maxPending + 1 {
397+ s.admit(&request{sender: c.UniqueName(), serial: uint32(i + 1), obj: objIntrospect, method: mGetWindows})
398+ }
399+ // Closing the real connection bounds reads without concurrently using its
400+ // encoder/decoder. No reply means a failed test, not the CI-wide timeout.
401+ timer := time.AfterFunc(5*time.Second, func() { _ = c.Close() })
402+ defer timer.Stop()
389403 m, err := c.ReadMessage()
404+ for err == nil && m.Type == zerobus.TypeSignal {
405+ m, err = c.ReadMessage()
406+ }
390407 if err != nil {
391408 t.Fatal(err)
392409 }
393- if m.Type != zerobus.TypeError || m.ReplySerial != serials[maxPending] || m.ErrorName != errLimits {
410+ if m.Type != zerobus.TypeError || m.ReplySerial != maxPending+1 || m.ErrorName != errLimits {
394411 t.Fatalf("17th request: type %d error %q reply %d", m.Type, m.ErrorName, m.ReplySerial)
395412 }
396- close(h.peers.openGo)
413+ peer.state, peer.exe = callerKnown, "/usr/bin/grim"
414+ s.flush(peer)
397415 for range maxPending {
398416 m, err := c.ReadMessage()
399417 if err != nil {
@@ -374,26 +374,44 @@ func TestNonOwnerIsDenied(t *testing.T) {
374 // full, the next one gets LimitsExceeded, and the parked ones are answered374 // full, the next one gets LimitsExceeded, and the parked ones are answered
375 // once the check ends.375 // once the check ends.
376 func TestPendingTableFull(t *testing.T) {376 func TestPendingTableFull(t *testing.T) {
377- h := newSvcHarness(t, harnessOpts{holdOp: true})377+ // Drive admission on this goroutine, delaying the caller-check transition
378- c := h.client()378+ // rather than blocking Open on the service owner. A PID reply may arrive
379- h.waitName(c, nameIntrospect)379+ // before all calls do; blocking Open then prevents the owner filling the
380- serials := make([]uint32, 0, maxPending+1)380+ // table or answering the overflow at all.
381- for range maxPending + 1 {381+ addr, _ := startBus(t)
382- c.NewCall(nameIntrospect, pathIntrospect, ifaceIntrospect, "GetWindows", "")382+ c, err := zerobus.Dial(addr)
383- serial, err := c.Send()383+ if err != nil {
384- if err != nil {384+ t.Fatal(err)
385- t.Fatal(err)
386- }
387- serials = append(serials, serial)
388 }385 }
386+ t.Cleanup(func() { _ = c.Close() })
387+ server, err := zerobus.Dial(addr)
388+ if err != nil {
389+ t.Fatal(err)
390+ }
391+ t.Cleanup(func() { _ = server.Close() })
392+ s := New(captureallow.Builtin, nil, testLog(&logBuf{}))
393+ s.conn = server
394+ peer := &s.callers[0]
395+ *peer = caller{state: callerPid, name: c.UniqueName()}
396+ for i := range maxPending + 1 {
397+ s.admit(&request{sender: c.UniqueName(), serial: uint32(i + 1), obj: objIntrospect, method: mGetWindows})
398+ }
399+ // Closing the real connection bounds reads without concurrently using its
400+ // encoder/decoder. No reply means a failed test, not the CI-wide timeout.
401+ timer := time.AfterFunc(5*time.Second, func() { _ = c.Close() })
402+ defer timer.Stop()
389 m, err := c.ReadMessage()403 m, err := c.ReadMessage()
404+ for err == nil && m.Type == zerobus.TypeSignal {
405+ m, err = c.ReadMessage()
406+ }
390 if err != nil {407 if err != nil {
391 t.Fatal(err)408 t.Fatal(err)
392 }409 }
393- if m.Type != zerobus.TypeError || m.ReplySerial != serials[maxPending] || m.ErrorName != errLimits {410+ if m.Type != zerobus.TypeError || m.ReplySerial != maxPending+1 || m.ErrorName != errLimits {
394 t.Fatalf("17th request: type %d error %q reply %d", m.Type, m.ErrorName, m.ReplySerial)411 t.Fatalf("17th request: type %d error %q reply %d", m.Type, m.ErrorName, m.ReplySerial)
395 }412 }
396- close(h.peers.openGo)413+ peer.state, peer.exe = callerKnown, "/usr/bin/grim"
414+ s.flush(peer)
397 for range maxPending {415 for range maxPending {
398 m, err := c.ReadMessage()416 m, err := c.ReadMessage()
399 if err != nil {417 if err != nil {
M internal/adapters/screencast/harness_test.go +4 -12
@@ -71,12 +71,11 @@ func testLog(out *logBuf) zerowrap.Logger {
7171 return zerowrap.New(zerowrap.Config{Level: "debug", Format: "json", Output: out}).WithField("component", "screencast")
7272 }
7373
74-// peers is what the mocked peerResolver answers: the executable of every
75-// peer, and a hook to hold Open.
74+// peers is what the mocked peerResolver answers: the executable and pid
75+// of every peer.
7676 type peers struct {
7777 exe atomic.Value // string
7878 exeNow func() string
79- openGo chan struct{} // when set, Open waits for it
8079 pid atomic.Int64
8180 openErr error
8281 exePid func(open int) int // pid Exe reports; the opened one when nil
@@ -118,9 +117,8 @@ type svcHarness struct {
118117 }
119118
120119 type harnessOpts struct {
121- allow *captureallow.Policy
122- noPW bool
123- holdOp bool
120+ allow *captureallow.Policy
121+ noPW bool
124122 // drain overrides how long the stopping service waits for frames.
125123 drain time.Duration
126124 // iterate runs inside every PipeWire loop iteration, on the owner.
@@ -132,9 +130,6 @@ func newSvcHarness(t *testing.T, o harnessOpts) *svcHarness {
132130 h := &svcHarness{t: t, cmds: make(chan ports.CastCommand, 64), peers: &peers{}, log: &logBuf{}, done: make(chan struct{}), closed: make(chan struct{}, 8)}
133131 h.addr, h.killBus = startBus(t)
134132 h.peers.exe.Store("/usr/bin/grim")
135- if o.holdOp {
136- h.peers.openGo = make(chan struct{})
137- }
138133 allow := o.allow
139134 if allow == nil {
140135 allow = captureallow.Builtin()
@@ -147,9 +142,6 @@ func newSvcHarness(t *testing.T, o harnessOpts) *svcHarness {
147142
148143 res := newMockpeerResolver(t)
149144 res.EXPECT().Open(mock.Anything).RunAndReturn(func(pid int) (*os.File, error) {
150- if h.peers.openGo != nil {
151- <-h.peers.openGo
152- }
153145 if h.peers.openErr != nil {
154146 return nil, h.peers.openErr
155147 }
@@ -71,12 +71,11 @@ func testLog(out *logBuf) zerowrap.Logger {
71 return zerowrap.New(zerowrap.Config{Level: "debug", Format: "json", Output: out}).WithField("component", "screencast")71 return zerowrap.New(zerowrap.Config{Level: "debug", Format: "json", Output: out}).WithField("component", "screencast")
72 }72 }
73 73
74-// peers is what the mocked peerResolver answers: the executable of every74+// peers is what the mocked peerResolver answers: the executable and pid
75-// peer, and a hook to hold Open.75+// of every peer.
76 type peers struct {76 type peers struct {
77 exe atomic.Value // string77 exe atomic.Value // string
78 exeNow func() string78 exeNow func() string
79- openGo chan struct{} // when set, Open waits for it
80 pid atomic.Int6479 pid atomic.Int64
81 openErr error80 openErr error
82 exePid func(open int) int // pid Exe reports; the opened one when nil81 exePid func(open int) int // pid Exe reports; the opened one when nil
@@ -118,9 +117,8 @@ type svcHarness struct {
118 }117 }
119 118
120 type harnessOpts struct {119 type harnessOpts struct {
121- allow *captureallow.Policy120+ allow *captureallow.Policy
122- noPW bool121+ noPW bool
123- holdOp bool
124 // drain overrides how long the stopping service waits for frames.122 // drain overrides how long the stopping service waits for frames.
125 drain time.Duration123 drain time.Duration
126 // iterate runs inside every PipeWire loop iteration, on the owner.124 // iterate runs inside every PipeWire loop iteration, on the owner.
@@ -132,9 +130,6 @@ func newSvcHarness(t *testing.T, o harnessOpts) *svcHarness {
132 h := &svcHarness{t: t, cmds: make(chan ports.CastCommand, 64), peers: &peers{}, log: &logBuf{}, done: make(chan struct{}), closed: make(chan struct{}, 8)}130 h := &svcHarness{t: t, cmds: make(chan ports.CastCommand, 64), peers: &peers{}, log: &logBuf{}, done: make(chan struct{}), closed: make(chan struct{}, 8)}
133 h.addr, h.killBus = startBus(t)131 h.addr, h.killBus = startBus(t)
134 h.peers.exe.Store("/usr/bin/grim")132 h.peers.exe.Store("/usr/bin/grim")
135- if o.holdOp {
136- h.peers.openGo = make(chan struct{})
137- }
138 allow := o.allow133 allow := o.allow
139 if allow == nil {134 if allow == nil {
140 allow = captureallow.Builtin()135 allow = captureallow.Builtin()
@@ -147,9 +142,6 @@ func newSvcHarness(t *testing.T, o harnessOpts) *svcHarness {
147 142
148 res := newMockpeerResolver(t)143 res := newMockpeerResolver(t)
149 res.EXPECT().Open(mock.Anything).RunAndReturn(func(pid int) (*os.File, error) {144 res.EXPECT().Open(mock.Anything).RunAndReturn(func(pid int) (*os.File, error) {
150- if h.peers.openGo != nil {
151- <-h.peers.openGo
152- }
153 if h.peers.openErr != nil {145 if h.peers.openErr != nil {
154 return nil, h.peers.openErr146 return nil, h.peers.openErr
155 }147 }

Keyboard shortcuts

Single-key shortcuts are on. ctrl k and esc work whatever this says.

ctrl k :
go to a section, branch, tag, file or repository
?
this list
j k
move the row cursor down / up
enter
open the row under the cursor; on Files, a directory opens or closes in place
↑ ↓ ← → home end
in the file tree (focus on a row): previous / next row, close / open a directory or go to its parent / first entry, first / last row. They never open a file
l h
on Files: open a directory or a file / close a directory or go up to its parent, then to the parent page. Elsewhere: into the side panel (on a merge request, its files) / back to the content; there j k move between its controls
g g G
first / last row
/
filter the list; on Files, find a file anywhere in the repository (esc clears, enter opens the best match)
b
switch branch or tag
g s g f g c g b g m g i g r g a
tabs: summary, files, commits, branches & tags, merge requests, issues, releases, actions (when available)
g h
home
w
wide mode: use the whole width of the screen, or not (remembered)
y
copy a permalink pinned to the commit
d u x
diff: split / unified / collapse or expand all
e E
diff: the whole file in view / every file, with the lines the hunks hide
n
merge request: next note in the diff
esc
close a dialog, leave a field

Branches 26

Tags 10

↑↓ moveenter openesc close