detached 0516c4d

fix(screencast): wait without a deadline for frames in flight when the bus is lost

A reconnect follows a bus loss while the wayland server still answers every
CastFrame, so shutdown waits for the frames and destroys every source: none
is left holding a slot of the process-wide video source table. The bounded
wait applies only when the service itself is stopping.
commit
0516c4d32c3e3d8047ce810376d54a5e55543c70
author
brice · · 2026-10-14 21:36 UTC
committer
brice · · 2026-10-14 21:39 UTC
parent
7a469c8
signature
GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email
4 files · +118 -52
M internal/adapters/screencast/harness_test.go +9 -2
@@ -112,14 +112,17 @@ type svcHarness struct {
112112 log *logBuf
113113 done chan struct{}
114114
115- killBus func() // loses the bus: the daemon dies
116- closed chan struct{} // a PipeWire connection was closed
115+ killBus func() // loses the bus: the daemon dies
116+ cancel context.CancelFunc // stops the service, as the process exit does
117+ closed chan struct{} // a PipeWire connection was closed
117118 }
118119
119120 type harnessOpts struct {
120121 allow *captureallow.Policy
121122 noPW bool
122123 holdOp bool
124+ // drain overrides how long the stopping service waits for frames.
125+ drain time.Duration
123126 // iterate runs inside every PipeWire loop iteration, on the owner.
124127 iterate func()
125128 }
@@ -138,6 +141,9 @@ func newSvcHarness(t *testing.T, o harnessOpts) *svcHarness {
138141 }
139142 h.svc = New(func() *captureallow.Policy { return allow }, h.cmds, testLog(h.log))
140143 h.svc.address = h.addr
144+ if o.drain > 0 {
145+ h.svc.drain = o.drain
146+ }
141147
142148 res := newMockpeerResolver(t)
143149 res.EXPECT().Open(mock.Anything).RunAndReturn(func(pid int) (*os.File, error) {
@@ -201,6 +207,7 @@ func newSvcHarness(t *testing.T, o harnessOpts) *svcHarness {
201207 h.svc.pw = pwOpen
202208
203209 ctx, cancel := context.WithCancel(context.Background())
210+ h.cancel = cancel
204211 go func() { defer close(h.done); h.svc.Run(ctx, 10*time.Millisecond) }()
205212 t.Cleanup(func() {
206213 cancel()
@@ -112,14 +112,17 @@ type svcHarness struct {
112 log *logBuf112 log *logBuf
113 done chan struct{}113 done chan struct{}
114 114
115- killBus func() // loses the bus: the daemon dies115+ killBus func() // loses the bus: the daemon dies
116- closed chan struct{} // a PipeWire connection was closed116+ cancel context.CancelFunc // stops the service, as the process exit does
117+ closed chan struct{} // a PipeWire connection was closed
117 }118 }
118 119
119 type harnessOpts struct {120 type harnessOpts struct {
120 allow *captureallow.Policy121 allow *captureallow.Policy
121 noPW bool122 noPW bool
122 holdOp bool123 holdOp bool
124+ // drain overrides how long the stopping service waits for frames.
125+ drain time.Duration
123 // iterate runs inside every PipeWire loop iteration, on the owner.126 // iterate runs inside every PipeWire loop iteration, on the owner.
124 iterate func()127 iterate func()
125 }128 }
@@ -138,6 +141,9 @@ func newSvcHarness(t *testing.T, o harnessOpts) *svcHarness {
138 }141 }
139 h.svc = New(func() *captureallow.Policy { return allow }, h.cmds, testLog(h.log))142 h.svc = New(func() *captureallow.Policy { return allow }, h.cmds, testLog(h.log))
140 h.svc.address = h.addr143 h.svc.address = h.addr
144+ if o.drain > 0 {
145+ h.svc.drain = o.drain
146+ }
141 147
142 res := newMockpeerResolver(t)148 res := newMockpeerResolver(t)
143 res.EXPECT().Open(mock.Anything).RunAndReturn(func(pid int) (*os.File, error) {149 res.EXPECT().Open(mock.Anything).RunAndReturn(func(pid int) (*os.File, error) {
@@ -201,6 +207,7 @@ func newSvcHarness(t *testing.T, o harnessOpts) *svcHarness {
201 h.svc.pw = pwOpen207 h.svc.pw = pwOpen
202 208
203 ctx, cancel := context.WithCancel(context.Background())209 ctx, cancel := context.WithCancel(context.Background())
210+ h.cancel = cancel
204 go func() { defer close(h.done); h.svc.Run(ctx, 10*time.Millisecond) }()211 go func() { defer close(h.done); h.svc.Run(ctx, 10*time.Millisecond) }()
205 t.Cleanup(func() {212 t.Cleanup(func() {
206 cancel()213 cancel()
M internal/adapters/screencast/producer_test.go +39 -19
@@ -1,7 +1,9 @@
11 package screencast
22
33 import (
4+ "context"
45 "errors"
6+ "sync/atomic"
57 "testing"
68 "time"
79
@@ -368,7 +370,7 @@ func TestShutdownClosesStreams(t *testing.T) {
368370 var order []string
369371 p.src.EXPECT().Destroy().Run(func() { order = append(order, "destroy") }).Once()
370372 p.core.EXPECT().Close().Run(func() { order = append(order, "close") }).Once()
371- p.s.shutdown()
373+ p.s.shutdown(context.Background())
372374 if c := p.cmd(); c.Kind != ports.CastClose || c.ID != 1 {
373375 t.Fatalf("%+v", c)
374376 }
@@ -388,7 +390,7 @@ func TestShutdownClosesStreamWithoutSource(t *testing.T) {
388390 t.Fatalf("%+v", c)
389391 }
390392 p.core.EXPECT().Close().Return().Once()
391- p.s.shutdown()
393+ p.s.shutdown(context.Background())
392394 if c := p.cmd(); c.Kind != ports.CastClose || c.ID != 1 {
393395 t.Fatalf("%+v", c)
394396 }
@@ -397,36 +399,51 @@ func TestShutdownClosesStreamWithoutSource(t *testing.T) {
397399 }
398400 }
399401
400-// A frame in flight at shutdown is waited for: nothing is destroyed before
401-// its CastFrameDone, then the buffer is released and the node destroyed.
402+// A frame in flight at shutdown is waited for: nothing is released or
403+// destroyed before its CastFrameDone, then the buffer is released, the node
404+// destroyed and PipeWire closed, in that order. The wait has no deadline
405+// while the service runs on (drain is far shorter than the wait here).
402406 func TestShutdownWaitsForFrameInFlight(t *testing.T) {
403407 p := newProducer(t)
408+ p.s.drain = time.Nanosecond
404409 p.streaming()
405410 p.src.EXPECT().Dequeue().Return(p.buf).Once()
406411 p.s.pump(p.st, p.now)
407412 p.cmd()
413+ var delivered atomic.Bool
408414 var order []string
409- p.src.EXPECT().Release(p.buf).Run(func(*pipewire.VideoBuffer) { order = append(order, "release") }).Once()
410- p.src.EXPECT().Destroy().Run(func() { order = append(order, "destroy") }).Once()
411- p.core.EXPECT().Close().Run(func() { order = append(order, "close") }).Once()
412- go func() {
413- time.Sleep(30 * time.Millisecond)
414- p.s.Push(ports.CastEvent{Kind: ports.CastFrameDone, ID: 1, Buf: 3})
415- }()
416- start := time.Now()
417- p.s.shutdown()
418- if time.Since(start) < 30*time.Millisecond {
419- t.Fatal("shutdown did not wait for the frame")
415+ step := func(name string) {
416+ if !delivered.Load() {
417+ t.Errorf("%s before the CastFrameDone", name)
418+ }
419+ order = append(order, name)
420420 }
421- if c := p.cmd(); c.Kind != ports.CastClose {
422- t.Fatalf("%+v", c)
421+ p.src.EXPECT().Release(p.buf).Run(func(*pipewire.VideoBuffer) { step("release") }).Once()
422+ p.src.EXPECT().Destroy().Run(func() { step("destroy") }).Once()
423+ p.core.EXPECT().Close().Run(func() { step("close") }).Once()
424+ done := make(chan struct{})
425+ go func() { defer close(done); p.s.shutdown(context.Background()) }()
426+ select {
427+ case c := <-p.cmds:
428+ if c.Kind != ports.CastClose {
429+ t.Errorf("%+v", c)
430+ }
431+ case <-time.After(5 * time.Second):
432+ t.Fatal("no CastClose")
433+ }
434+ delivered.Store(true)
435+ p.s.Push(ports.CastEvent{Kind: ports.CastFrameDone, ID: 1, Buf: 3})
436+ select {
437+ case <-done:
438+ case <-time.After(5 * time.Second):
439+ t.Fatal("shutdown did not return")
423440 }
424441 if len(order) != 3 || order[0] != "release" || order[1] != "destroy" || order[2] != "close" {
425442 t.Fatalf("order %v", order)
426443 }
427444 }
428445
429-// When the frame never comes back, nothing is destroyed or closed (the
446+// When the service itself is stopping and the frame never comes back, nothing is destroyed or closed (the
430447 // source's handler and its buffer are still in use), the storage the
431448 // handler points into is left untouched and abandoned, and the service has
432449 // a fresh table: repeated reconnects do not exhaust it.
@@ -438,7 +455,10 @@ func TestShutdownTimeoutKeepsHandlerStateAndFreesTable(t *testing.T) {
438455 p.s.pump(p.st, p.now)
439456 p.cmd()
440457 old := p.s.sessions
441- p.s.shutdown() // no Destroy, Release or Close is expected
458+ // The service itself is stopping: the wait is bounded.
459+ stopped, stop := context.WithCancel(context.Background())
460+ stop()
461+ p.s.shutdown(stopped) // no Destroy, Release or Close is expected
442462 if c := p.cmd(); c.Kind != ports.CastClose {
443463 t.Fatalf("%+v", c)
444464 }
@@ -1,7 +1,9 @@
1 package screencast1 package screencast
2 2
3 import (3 import (
4+ "context"
4 "errors"5 "errors"
6+ "sync/atomic"
5 "testing"7 "testing"
6 "time"8 "time"
7 9
@@ -368,7 +370,7 @@ func TestShutdownClosesStreams(t *testing.T) {
368 var order []string370 var order []string
369 p.src.EXPECT().Destroy().Run(func() { order = append(order, "destroy") }).Once()371 p.src.EXPECT().Destroy().Run(func() { order = append(order, "destroy") }).Once()
370 p.core.EXPECT().Close().Run(func() { order = append(order, "close") }).Once()372 p.core.EXPECT().Close().Run(func() { order = append(order, "close") }).Once()
371- p.s.shutdown()373+ p.s.shutdown(context.Background())
372 if c := p.cmd(); c.Kind != ports.CastClose || c.ID != 1 {374 if c := p.cmd(); c.Kind != ports.CastClose || c.ID != 1 {
373 t.Fatalf("%+v", c)375 t.Fatalf("%+v", c)
374 }376 }
@@ -388,7 +390,7 @@ func TestShutdownClosesStreamWithoutSource(t *testing.T) {
388 t.Fatalf("%+v", c)390 t.Fatalf("%+v", c)
389 }391 }
390 p.core.EXPECT().Close().Return().Once()392 p.core.EXPECT().Close().Return().Once()
391- p.s.shutdown()393+ p.s.shutdown(context.Background())
392 if c := p.cmd(); c.Kind != ports.CastClose || c.ID != 1 {394 if c := p.cmd(); c.Kind != ports.CastClose || c.ID != 1 {
393 t.Fatalf("%+v", c)395 t.Fatalf("%+v", c)
394 }396 }
@@ -397,36 +399,51 @@ func TestShutdownClosesStreamWithoutSource(t *testing.T) {
397 }399 }
398 }400 }
399 401
400-// A frame in flight at shutdown is waited for: nothing is destroyed before402+// A frame in flight at shutdown is waited for: nothing is released or
401-// its CastFrameDone, then the buffer is released and the node destroyed.403+// destroyed before its CastFrameDone, then the buffer is released, the node
404+// destroyed and PipeWire closed, in that order. The wait has no deadline
405+// while the service runs on (drain is far shorter than the wait here).
402 func TestShutdownWaitsForFrameInFlight(t *testing.T) {406 func TestShutdownWaitsForFrameInFlight(t *testing.T) {
403 p := newProducer(t)407 p := newProducer(t)
408+ p.s.drain = time.Nanosecond
404 p.streaming()409 p.streaming()
405 p.src.EXPECT().Dequeue().Return(p.buf).Once()410 p.src.EXPECT().Dequeue().Return(p.buf).Once()
406 p.s.pump(p.st, p.now)411 p.s.pump(p.st, p.now)
407 p.cmd()412 p.cmd()
413+ var delivered atomic.Bool
408 var order []string414 var order []string
409- p.src.EXPECT().Release(p.buf).Run(func(*pipewire.VideoBuffer) { order = append(order, "release") }).Once()415+ step := func(name string) {
410- p.src.EXPECT().Destroy().Run(func() { order = append(order, "destroy") }).Once()416+ if !delivered.Load() {
411- p.core.EXPECT().Close().Run(func() { order = append(order, "close") }).Once()417+ t.Errorf("%s before the CastFrameDone", name)
412- go func() {418+ }
413- time.Sleep(30 * time.Millisecond)419+ order = append(order, name)
414- p.s.Push(ports.CastEvent{Kind: ports.CastFrameDone, ID: 1, Buf: 3})
415- }()
416- start := time.Now()
417- p.s.shutdown()
418- if time.Since(start) < 30*time.Millisecond {
419- t.Fatal("shutdown did not wait for the frame")
420 }420 }
421- if c := p.cmd(); c.Kind != ports.CastClose {421+ p.src.EXPECT().Release(p.buf).Run(func(*pipewire.VideoBuffer) { step("release") }).Once()
422- t.Fatalf("%+v", c)422+ p.src.EXPECT().Destroy().Run(func() { step("destroy") }).Once()
423+ p.core.EXPECT().Close().Run(func() { step("close") }).Once()
424+ done := make(chan struct{})
425+ go func() { defer close(done); p.s.shutdown(context.Background()) }()
426+ select {
427+ case c := <-p.cmds:
428+ if c.Kind != ports.CastClose {
429+ t.Errorf("%+v", c)
430+ }
431+ case <-time.After(5 * time.Second):
432+ t.Fatal("no CastClose")
433+ }
434+ delivered.Store(true)
435+ p.s.Push(ports.CastEvent{Kind: ports.CastFrameDone, ID: 1, Buf: 3})
436+ select {
437+ case <-done:
438+ case <-time.After(5 * time.Second):
439+ t.Fatal("shutdown did not return")
423 }440 }
424 if len(order) != 3 || order[0] != "release" || order[1] != "destroy" || order[2] != "close" {441 if len(order) != 3 || order[0] != "release" || order[1] != "destroy" || order[2] != "close" {
425 t.Fatalf("order %v", order)442 t.Fatalf("order %v", order)
426 }443 }
427 }444 }
428 445
429-// When the frame never comes back, nothing is destroyed or closed (the446+// When the service itself is stopping and the frame never comes back, nothing is destroyed or closed (the
430 // source's handler and its buffer are still in use), the storage the447 // source's handler and its buffer are still in use), the storage the
431 // handler points into is left untouched and abandoned, and the service has448 // handler points into is left untouched and abandoned, and the service has
432 // a fresh table: repeated reconnects do not exhaust it.449 // a fresh table: repeated reconnects do not exhaust it.
@@ -438,7 +455,10 @@ func TestShutdownTimeoutKeepsHandlerStateAndFreesTable(t *testing.T) {
438 p.s.pump(p.st, p.now)455 p.s.pump(p.st, p.now)
439 p.cmd()456 p.cmd()
440 old := p.s.sessions457 old := p.s.sessions
441- p.s.shutdown() // no Destroy, Release or Close is expected458+ // The service itself is stopping: the wait is bounded.
459+ stopped, stop := context.WithCancel(context.Background())
460+ stop()
461+ p.s.shutdown(stopped) // no Destroy, Release or Close is expected
442 if c := p.cmd(); c.Kind != ports.CastClose {462 if c := p.cmd(); c.Kind != ports.CastClose {
443 t.Fatalf("%+v", c)463 t.Fatalf("%+v", c)
444 }464 }
M internal/adapters/screencast/screencast.go +29 -16
@@ -52,7 +52,8 @@ const (
5252 // shotTimeout bounds a nefercap run.
5353 shotTimeout = 60 * time.Second
5454
55- // shutdownWait bounds the wait for frames still in flight at shutdown.
55+ // shutdownWait bounds the wait for frames still in flight at shutdown,
56+ // once the service itself is stopping.
5657 shutdownWait = 2 * time.Second
5758
5859 // shutdownPoll is how often shutdown looks again for the events that
@@ -99,9 +100,9 @@ type Service struct {
99100 haveInv bool
100101 callers [maxCallers]caller
101102 pending [maxPending]pending
102- // sessions is a pointer so that a shutdown that had to leave a PipeWire
103- // source behind can abandon the table: the source's handler still
104- // points into it, so it is never reset or reused.
103+ // sessions is a pointer so that a shutdown at process exit that had to
104+ // leave a PipeWire source behind can abandon the table: the source's
105+ // handler still points into it, so it is never reset or reused.
105106 sessions *[maxSessions]session
106107 nextSess uint32
107108 nextStrm uint32
@@ -111,7 +112,7 @@ type Service struct {
111112 shotSeq uint64 // ID of the last screenshot job; never reset
112113 fatal error
113114 jobs sync.WaitGroup
114- drain time.Duration // how long shutdown waits for frames in flight
115+ drain time.Duration // how long shutdown waits for frames once the service stops
115116 }
116117
117118 // New creates the service. policy returns the capture allowlist in force;
@@ -207,7 +208,7 @@ func (s *Service) Run(ctx context.Context, retry time.Duration) {
207208 }
208209
209210 // attempt is one connection of the service to the buses.
210-func (s *Service) attempt(ctx context.Context) (err error) {
211+func (s *Service) attempt(parent context.Context) (err error) {
211212 // The PipeWire loop belongs to this OS thread for its whole life.
212213 runtime.LockOSThread()
213214 defer runtime.UnlockOSThread()
@@ -217,14 +218,14 @@ func (s *Service) attempt(ctx context.Context) (err error) {
217218 }
218219 defer func() { _ = conn.Close() }()
219220 // The screenshot job of this attempt ends with it.
220- ctx, cancel := context.WithCancel(ctx)
221+ ctx, cancel := context.WithCancel(parent)
221222 defer cancel()
222223 s.reset(ctx, conn)
223224 if s.pwc, err = s.pw.Open(); err != nil {
224225 s.pwc = nil
225226 s.log.Warn().Err(err).Msg("PipeWire unavailable: screen casting is not offered")
226227 }
227- defer s.shutdown()
228+ defer s.shutdown(parent)
228229 if err = s.acquire(); err != nil {
229230 return err
230231 }
@@ -346,16 +347,25 @@ func (s *Service) service() {
346347 // resources: CastClose goes to the wayland server, a frame still in flight
347348 // is waited for (its buffer is lent to the wayland server), then the source
348349 // is destroyed and its buffers released. Only then is PipeWire closed.
350+// parent is the context the service runs under; the attempt's own context
351+// may have ended for another reason.
349352 //
350-// When a frame does not come back in time, the source cannot be destroyed
351-// safely. It is left as it is, with its loop: the sessions table that its
352-// handler points into is abandoned (never reset or reused) and a new one
353-// takes its place, so later attempts neither touch that storage nor run out
354-// of slots.
355-func (s *Service) shutdown() {
353+// While parent is alive the attempt ended by itself (the bus was lost) and a
354+// new one follows. The wayland server is alive and answers every CastFrame
355+// with a CastFrameDone, so the wait for the frames in flight has no
356+// deadline: a source left behind would keep its slot in the process-wide
357+// table of PipeWire video sources for good, and repeated reconnects would
358+// exhaust it. If parent ends meanwhile, the wait is bounded as below.
359+//
360+// Once parent has ended, the process is going away and the wait for the
361+// frames in flight is bounded by s.drain. When a frame does not come back in
362+// time, the source cannot be destroyed safely. It is left as it is, with its
363+// loop, to the process exit. The sessions table that its handler points into
364+// is abandoned (never reset or reused) and a new one takes its place.
365+func (s *Service) shutdown(parent context.Context) {
356366 s.stopShot()
357367 s.forEachSession(func(sess *session) { s.closeSession(sess, true) })
358- deadline := time.Now().Add(s.drain)
368+ var deadline time.Time // set once parent ended
359369 for {
360370 evs, _, _, _ := s.take()
361371 for i := range evs {
@@ -366,7 +376,10 @@ func (s *Service) shutdown() {
366376 if !s.streamsLive() {
367377 break
368378 }
369- if !time.Now().Before(deadline) {
379+ if deadline.IsZero() && parent.Err() != nil {
380+ deadline = time.Now().Add(s.drain)
381+ }
382+ if !deadline.IsZero() && !time.Now().Before(deadline) {
370383 s.log.Warn().Int("streams", s.streamCount()).Msg("frames still in flight at shutdown; PipeWire resources are left to the process exit")
371384 s.sessions = new([maxSessions]session)
372385 s.pwc = nil
@@ -52,7 +52,8 @@ const (
52 // shotTimeout bounds a nefercap run.52 // shotTimeout bounds a nefercap run.
53 shotTimeout = 60 * time.Second53 shotTimeout = 60 * time.Second
54 54
55- // shutdownWait bounds the wait for frames still in flight at shutdown.55+ // shutdownWait bounds the wait for frames still in flight at shutdown,
56+ // once the service itself is stopping.
56 shutdownWait = 2 * time.Second57 shutdownWait = 2 * time.Second
57 58
58 // shutdownPoll is how often shutdown looks again for the events that59 // shutdownPoll is how often shutdown looks again for the events that
@@ -99,9 +100,9 @@ type Service struct {
99 haveInv bool100 haveInv bool
100 callers [maxCallers]caller101 callers [maxCallers]caller
101 pending [maxPending]pending102 pending [maxPending]pending
102- // sessions is a pointer so that a shutdown that had to leave a PipeWire103+ // sessions is a pointer so that a shutdown at process exit that had to
103- // source behind can abandon the table: the source's handler still104+ // leave a PipeWire source behind can abandon the table: the source's
104- // points into it, so it is never reset or reused.105+ // handler still points into it, so it is never reset or reused.
105 sessions *[maxSessions]session106 sessions *[maxSessions]session
106 nextSess uint32107 nextSess uint32
107 nextStrm uint32108 nextStrm uint32
@@ -111,7 +112,7 @@ type Service struct {
111 shotSeq uint64 // ID of the last screenshot job; never reset112 shotSeq uint64 // ID of the last screenshot job; never reset
112 fatal error113 fatal error
113 jobs sync.WaitGroup114 jobs sync.WaitGroup
114- drain time.Duration // how long shutdown waits for frames in flight115+ drain time.Duration // how long shutdown waits for frames once the service stops
115 }116 }
116 117
117 // New creates the service. policy returns the capture allowlist in force;118 // New creates the service. policy returns the capture allowlist in force;
@@ -207,7 +208,7 @@ func (s *Service) Run(ctx context.Context, retry time.Duration) {
207 }208 }
208 209
209 // attempt is one connection of the service to the buses.210 // attempt is one connection of the service to the buses.
210-func (s *Service) attempt(ctx context.Context) (err error) {211+func (s *Service) attempt(parent context.Context) (err error) {
211 // The PipeWire loop belongs to this OS thread for its whole life.212 // The PipeWire loop belongs to this OS thread for its whole life.
212 runtime.LockOSThread()213 runtime.LockOSThread()
213 defer runtime.UnlockOSThread()214 defer runtime.UnlockOSThread()
@@ -217,14 +218,14 @@ func (s *Service) attempt(ctx context.Context) (err error) {
217 }218 }
218 defer func() { _ = conn.Close() }()219 defer func() { _ = conn.Close() }()
219 // The screenshot job of this attempt ends with it.220 // The screenshot job of this attempt ends with it.
220- ctx, cancel := context.WithCancel(ctx)221+ ctx, cancel := context.WithCancel(parent)
221 defer cancel()222 defer cancel()
222 s.reset(ctx, conn)223 s.reset(ctx, conn)
223 if s.pwc, err = s.pw.Open(); err != nil {224 if s.pwc, err = s.pw.Open(); err != nil {
224 s.pwc = nil225 s.pwc = nil
225 s.log.Warn().Err(err).Msg("PipeWire unavailable: screen casting is not offered")226 s.log.Warn().Err(err).Msg("PipeWire unavailable: screen casting is not offered")
226 }227 }
227- defer s.shutdown()228+ defer s.shutdown(parent)
228 if err = s.acquire(); err != nil {229 if err = s.acquire(); err != nil {
229 return err230 return err
230 }231 }
@@ -346,16 +347,25 @@ func (s *Service) service() {
346 // resources: CastClose goes to the wayland server, a frame still in flight347 // resources: CastClose goes to the wayland server, a frame still in flight
347 // is waited for (its buffer is lent to the wayland server), then the source348 // is waited for (its buffer is lent to the wayland server), then the source
348 // is destroyed and its buffers released. Only then is PipeWire closed.349 // is destroyed and its buffers released. Only then is PipeWire closed.
350+// parent is the context the service runs under; the attempt's own context
351+// may have ended for another reason.
349 //352 //
350-// When a frame does not come back in time, the source cannot be destroyed353+// While parent is alive the attempt ended by itself (the bus was lost) and a
351-// safely. It is left as it is, with its loop: the sessions table that its354+// new one follows. The wayland server is alive and answers every CastFrame
352-// handler points into is abandoned (never reset or reused) and a new one355+// with a CastFrameDone, so the wait for the frames in flight has no
353-// takes its place, so later attempts neither touch that storage nor run out356+// deadline: a source left behind would keep its slot in the process-wide
354-// of slots.357+// table of PipeWire video sources for good, and repeated reconnects would
355-func (s *Service) shutdown() {358+// exhaust it. If parent ends meanwhile, the wait is bounded as below.
359+//
360+// Once parent has ended, the process is going away and the wait for the
361+// frames in flight is bounded by s.drain. When a frame does not come back in
362+// time, the source cannot be destroyed safely. It is left as it is, with its
363+// loop, to the process exit. The sessions table that its handler points into
364+// is abandoned (never reset or reused) and a new one takes its place.
365+func (s *Service) shutdown(parent context.Context) {
356 s.stopShot()366 s.stopShot()
357 s.forEachSession(func(sess *session) { s.closeSession(sess, true) })367 s.forEachSession(func(sess *session) { s.closeSession(sess, true) })
358- deadline := time.Now().Add(s.drain)368+ var deadline time.Time // set once parent ended
359 for {369 for {
360 evs, _, _, _ := s.take()370 evs, _, _, _ := s.take()
361 for i := range evs {371 for i := range evs {
@@ -366,7 +376,10 @@ func (s *Service) shutdown() {
366 if !s.streamsLive() {376 if !s.streamsLive() {
367 break377 break
368 }378 }
369- if !time.Now().Before(deadline) {379+ if deadline.IsZero() && parent.Err() != nil {
380+ deadline = time.Now().Add(s.drain)
381+ }
382+ if !deadline.IsZero() && !time.Now().Before(deadline) {
370 s.log.Warn().Int("streams", s.streamCount()).Msg("frames still in flight at shutdown; PipeWire resources are left to the process exit")383 s.log.Warn().Int("streams", s.streamCount()).Msg("frames still in flight at shutdown; PipeWire resources are left to the process exit")
371 s.sessions = new([maxSessions]session)384 s.sessions = new([maxSessions]session)
372 s.pwc = nil385 s.pwc = nil
M internal/adapters/screencast/shutdown_test.go +41 -15
@@ -1,6 +1,7 @@
11 package screencast
22
33 import (
4+ "sync/atomic"
45 "testing"
56 "time"
67
@@ -26,10 +27,10 @@ func (l *liveCast) loop(f func()) {
2627 l.h.svc.Push(ports.CastEvent{Kind: ports.CastDamage, ID: 1 << 40}) // wakes the owner
2728 }
2829
29-func startLiveCast(t *testing.T, withFrame bool) *liveCast {
30+func startLiveCast(t *testing.T, withFrame bool, drain time.Duration) *liveCast {
3031 t.Helper()
3132 l := &liveCast{onLoop: make(chan func(), 8), hdl: make(chan pipewire.VideoHandler, 1)}
32- l.h, _ = ready(t, harnessOpts{iterate: func() {
33+ l.h, _ = ready(t, harnessOpts{drain: drain, iterate: func() {
3334 select {
3435 case f := <-l.onLoop:
3536 f()
@@ -104,7 +105,7 @@ func waitClosed(t *testing.T, h *svcHarness) {
104105 // Losing the bus with a live stream and no frame in flight: the cast is
105106 // closed on the wayland side, the source destroyed, PipeWire closed.
106107 func TestBusLossTearsDownStream(t *testing.T) {
107- l := startLiveCast(t, false)
108+ l := startLiveCast(t, false, 0)
108109 l.src.EXPECT().Destroy().Return().Once()
109110 l.h.killBus()
110111 if cmd := recvCmd(t, l.h); cmd.Kind != ports.CastClose || cmd.ID != 1 {
@@ -114,25 +115,50 @@ func TestBusLossTearsDownStream(t *testing.T) {
114115 }
115116
116117 // With a frame in flight: CastClose goes out at once, but the source is
117-// destroyed only after the frame's CastFrameDone, and its buffer released.
118+// destroyed only after the frame's CastFrameDone, even when that comes later
119+// than drain: the service runs on, so nothing is left behind. The order is
120+// synchronized by channels: the buffer must be released before the node is
121+// destroyed, and PipeWire closed after both.
118122 func TestBusLossWaitsForFrameInFlight(t *testing.T) {
119- l := startLiveCast(t, true)
120- if cmd := func() ports.CastCommand { l.h.killBus(); return recvCmd(t, l.h) }(); cmd.Kind != ports.CastClose || cmd.ID != 1 {
123+ l := startLiveCast(t, true, time.Nanosecond)
124+ l.h.killBus()
125+ if cmd := recvCmd(t, l.h); cmd.Kind != ports.CastClose || cmd.ID != 1 {
121126 t.Fatalf("%+v", cmd)
122127 }
123- select {
124- case <-l.h.closed:
125- t.Fatal("PipeWire closed with a frame in flight")
126- case <-time.After(200 * time.Millisecond):
128+ var delivered atomic.Bool
129+ steps := make(chan string, 3)
130+ step := func(name string) {
131+ if !delivered.Load() {
132+ t.Errorf("%s before the CastFrameDone", name)
133+ }
134+ steps <- name
127135 }
128- released := make(chan struct{})
129- l.src.EXPECT().Release(l.buf).Run(func(*pipewire.VideoBuffer) { close(released) }).Once()
130- l.src.EXPECT().Destroy().Return().Once()
136+ l.src.EXPECT().Release(l.buf).Run(func(*pipewire.VideoBuffer) { step("release") }).Once()
137+ l.src.EXPECT().Destroy().Run(func() { step("destroy") }).Once()
138+ delivered.Store(true)
131139 l.h.svc.Push(ports.CastEvent{Kind: ports.CastFrameDone, ID: 1, Buf: 2})
132140 waitClosed(t, l.h)
141+ if len(steps) != 2 || <-steps != "release" || <-steps != "destroy" {
142+ t.Fatal("release and destroy did not run in order before PipeWire closed")
143+ }
144+}
145+
146+// The service itself stopping with a frame that is never answered returns
147+// within the bounded wait, and leaves the source and PipeWire alone.
148+func TestStopWithUnansweredFrameIsBounded(t *testing.T) {
149+ l := startLiveCast(t, true, 20*time.Millisecond)
150+ l.h.cancel()
151+ if cmd := recvCmd(t, l.h); cmd.Kind != ports.CastClose || cmd.ID != 1 {
152+ t.Fatalf("%+v", cmd)
153+ }
133154 select {
134- case <-released:
155+ case <-l.h.done:
156+ case <-time.After(5 * time.Second):
157+ t.Fatal("Run did not return")
158+ }
159+ select {
160+ case <-l.h.closed:
161+ t.Fatal("PipeWire closed under a source with a frame in flight")
135162 default:
136- t.Fatal("buffer not released")
137163 }
138164 }
@@ -1,6 +1,7 @@
1 package screencast1 package screencast
2 2
3 import (3 import (
4+ "sync/atomic"
4 "testing"5 "testing"
5 "time"6 "time"
6 7
@@ -26,10 +27,10 @@ func (l *liveCast) loop(f func()) {
26 l.h.svc.Push(ports.CastEvent{Kind: ports.CastDamage, ID: 1 << 40}) // wakes the owner27 l.h.svc.Push(ports.CastEvent{Kind: ports.CastDamage, ID: 1 << 40}) // wakes the owner
27 }28 }
28 29
29-func startLiveCast(t *testing.T, withFrame bool) *liveCast {30+func startLiveCast(t *testing.T, withFrame bool, drain time.Duration) *liveCast {
30 t.Helper()31 t.Helper()
31 l := &liveCast{onLoop: make(chan func(), 8), hdl: make(chan pipewire.VideoHandler, 1)}32 l := &liveCast{onLoop: make(chan func(), 8), hdl: make(chan pipewire.VideoHandler, 1)}
32- l.h, _ = ready(t, harnessOpts{iterate: func() {33+ l.h, _ = ready(t, harnessOpts{drain: drain, iterate: func() {
33 select {34 select {
34 case f := <-l.onLoop:35 case f := <-l.onLoop:
35 f()36 f()
@@ -104,7 +105,7 @@ func waitClosed(t *testing.T, h *svcHarness) {
104 // Losing the bus with a live stream and no frame in flight: the cast is105 // Losing the bus with a live stream and no frame in flight: the cast is
105 // closed on the wayland side, the source destroyed, PipeWire closed.106 // closed on the wayland side, the source destroyed, PipeWire closed.
106 func TestBusLossTearsDownStream(t *testing.T) {107 func TestBusLossTearsDownStream(t *testing.T) {
107- l := startLiveCast(t, false)108+ l := startLiveCast(t, false, 0)
108 l.src.EXPECT().Destroy().Return().Once()109 l.src.EXPECT().Destroy().Return().Once()
109 l.h.killBus()110 l.h.killBus()
110 if cmd := recvCmd(t, l.h); cmd.Kind != ports.CastClose || cmd.ID != 1 {111 if cmd := recvCmd(t, l.h); cmd.Kind != ports.CastClose || cmd.ID != 1 {
@@ -114,25 +115,50 @@ func TestBusLossTearsDownStream(t *testing.T) {
114 }115 }
115 116
116 // With a frame in flight: CastClose goes out at once, but the source is117 // With a frame in flight: CastClose goes out at once, but the source is
117-// destroyed only after the frame's CastFrameDone, and its buffer released.118+// destroyed only after the frame's CastFrameDone, even when that comes later
119+// than drain: the service runs on, so nothing is left behind. The order is
120+// synchronized by channels: the buffer must be released before the node is
121+// destroyed, and PipeWire closed after both.
118 func TestBusLossWaitsForFrameInFlight(t *testing.T) {122 func TestBusLossWaitsForFrameInFlight(t *testing.T) {
119- l := startLiveCast(t, true)123+ l := startLiveCast(t, true, time.Nanosecond)
120- if cmd := func() ports.CastCommand { l.h.killBus(); return recvCmd(t, l.h) }(); cmd.Kind != ports.CastClose || cmd.ID != 1 {124+ l.h.killBus()
125+ if cmd := recvCmd(t, l.h); cmd.Kind != ports.CastClose || cmd.ID != 1 {
121 t.Fatalf("%+v", cmd)126 t.Fatalf("%+v", cmd)
122 }127 }
123- select {128+ var delivered atomic.Bool
124- case <-l.h.closed:129+ steps := make(chan string, 3)
125- t.Fatal("PipeWire closed with a frame in flight")130+ step := func(name string) {
126- case <-time.After(200 * time.Millisecond):131+ if !delivered.Load() {
132+ t.Errorf("%s before the CastFrameDone", name)
133+ }
134+ steps <- name
127 }135 }
128- released := make(chan struct{})136+ l.src.EXPECT().Release(l.buf).Run(func(*pipewire.VideoBuffer) { step("release") }).Once()
129- l.src.EXPECT().Release(l.buf).Run(func(*pipewire.VideoBuffer) { close(released) }).Once()137+ l.src.EXPECT().Destroy().Run(func() { step("destroy") }).Once()
130- l.src.EXPECT().Destroy().Return().Once()138+ delivered.Store(true)
131 l.h.svc.Push(ports.CastEvent{Kind: ports.CastFrameDone, ID: 1, Buf: 2})139 l.h.svc.Push(ports.CastEvent{Kind: ports.CastFrameDone, ID: 1, Buf: 2})
132 waitClosed(t, l.h)140 waitClosed(t, l.h)
141+ if len(steps) != 2 || <-steps != "release" || <-steps != "destroy" {
142+ t.Fatal("release and destroy did not run in order before PipeWire closed")
143+ }
144+}
145+
146+// The service itself stopping with a frame that is never answered returns
147+// within the bounded wait, and leaves the source and PipeWire alone.
148+func TestStopWithUnansweredFrameIsBounded(t *testing.T) {
149+ l := startLiveCast(t, true, 20*time.Millisecond)
150+ l.h.cancel()
151+ if cmd := recvCmd(t, l.h); cmd.Kind != ports.CastClose || cmd.ID != 1 {
152+ t.Fatalf("%+v", cmd)
153+ }
133 select {154 select {
134- case <-released:155+ case <-l.h.done:
156+ case <-time.After(5 * time.Second):
157+ t.Fatal("Run did not return")
158+ }
159+ select {
160+ case <-l.h.closed:
161+ t.Fatal("PipeWire closed under a source with a frame in flight")
135 default:162 default:
136- t.Fatal("buffer not released")
137 }163 }
138 }164 }

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 23

Tags 10

↑↓ moveenter openesc close