6a15f35 fix(flash): bound the workspace lookup by the flash time budget · nefercap

M internal/adapters/flash/flash.go +3 -3


@@ -27,9 +27,9 @@ var styleSheet string
27	27		const (
28	28		namespace = "nefercap-flash"
29	29		step = 30 * time.Millisecond
30		-	// limit bounds the whole effect, setup included, so a slow compositor or
	30	+	// Limit bounds the whole effect, setup included, so a slow compositor or
31	31		// GPU never keeps the process alive.
32		-	limit = 2 * time.Second
	32	+	Limit = 2 * time.Second
33	33		)
34	34
35	35		// stepClasses are the opacity steps of style.css, from opaque to faint.

@@ -56,7 +56,7 @@ func show(ctx context.Context, output ports.Output, area ports.Region) error {
56	56		}
57	57		defer remove()
58	58
59		-	child, cancel := context.WithTimeout(ctx, limit)
	59	+	child, cancel := context.WithTimeout(ctx, Limit)
60	60		wake := make(chan struct{}, 1)
61	61		var wg sync.WaitGroup
62	62		wg.Go(func() { ticker(child, wake) })


@@ -27,9 +27,9 @@ var styleSheet string
27		const (	27		const (
28		namespace = "nefercap-flash"	28		namespace = "nefercap-flash"
29		step = 30 * time.Millisecond	29		step = 30 * time.Millisecond
30	-	// limit bounds the whole effect, setup included, so a slow compositor or	30	+	// Limit bounds the whole effect, setup included, so a slow compositor or
31		// GPU never keeps the process alive.	31		// GPU never keeps the process alive.
32	-	limit = 2 * time.Second	32	+	Limit = 2 * time.Second
33		)	33		)
34			34
35		// stepClasses are the opacity steps of style.css, from opaque to faint.	35		// stepClasses are the opacity steps of style.css, from opaque to faint.

@@ -56,7 +56,7 @@ func show(ctx context.Context, output ports.Output, area ports.Region) error {
56		}	56		}
57		defer remove()	57		defer remove()
58			58
59	-	child, cancel := context.WithTimeout(ctx, limit)	59	+	child, cancel := context.WithTimeout(ctx, Limit)
60		wake := make(chan struct{}, 1)	60		wake := make(chan struct{}, 1)
61		var wg sync.WaitGroup	61		var wg sync.WaitGroup
62		wg.Go(func() { ticker(child, wake) })	62		wg.Go(func() { ticker(child, wake) })

M internal/adapters/flash/flash_test.go +1 -1


@@ -69,5 +69,5 @@ func TestSmokeShow(t *testing.T) {
69	69		}
70	70		began := time.Now()
71	71		require.NoError(t, Show(context.Background(), ports.Output{ID: 1, Name: name}, ports.Region{X: 40, Y: 40, Width: 320, Height: 200}))
72		-	assert.Less(t, time.Since(began), limit+time.Second)
	72	+	assert.Less(t, time.Since(began), Limit+time.Second)
73	73		}


@@ -69,5 +69,5 @@ func TestSmokeShow(t *testing.T) {
69		}	69		}
70		began := time.Now()	70		began := time.Now()
71		require.NoError(t, Show(context.Background(), ports.Output{ID: 1, Name: name}, ports.Region{X: 40, Y: 40, Width: 320, Height: 200}))	71		require.NoError(t, Show(context.Background(), ports.Output{ID: 1, Name: name}, ports.Region{X: 40, Y: 40, Width: 320, Height: 200}))
72	-	assert.Less(t, time.Since(began), limit+time.Second)	72	+	assert.Less(t, time.Since(began), Limit+time.Second)
73		}	73		}

M internal/app/app.go +8 -1


@@ -112,8 +112,12 @@ func Run(ctx context.Context, options cli.Options, output io.Writer) (err error)
112	112		}
113	113
114	114		// confirmScreenshot flashes the captured area once the screenshot is done. The
115		-	// screenshot already succeeded, so a failed flash is only logged.
	115	+	// screenshot already succeeded, so a failed flash is only logged. One budget
	116	+	// bounds the workspace lookup and the flash, so a silent compositor cannot
	117	+	// keep the process alive.
116	118		func confirmScreenshot(ctx context.Context, source *wayland.Source, outputs []ports.Output, target ports.Target) {
	119	+	ctx, cancel := context.WithTimeout(ctx, flash.Limit)
	120	+	defer cancel()
117	121		for _, o := range outputs {
118	122		if o.ID != target.OutputID {
119	123		continue

@@ -121,6 +125,9 @@ func confirmScreenshot(ctx context.Context, source *wayland.Source, outputs []po
121	125		area := target.Region
122	126		if target.WorkspaceID != 0 {
123	127		area = recordedFrame(ctx, source, target)
	128	+	if ctx.Err() != nil {
	129	+	return // the budget went on the lookup
	130	+	}
124	131		}
125	132		if err := flash.Show(ctx, o, area); err != nil {
126	133		log := logging.For(ctx, "app")

 }
 // confirmScreenshot flashes the captured area once the screenshot is done. The
-// screenshot already succeeded, so a failed flash is only logged.
+// screenshot already succeeded, so a failed flash is only logged. One budget
+// bounds the workspace lookup and the flash, so a silent compositor cannot
+// keep the process alive.
 func confirmScreenshot(ctx context.Context, source *wayland.Source, outputs []ports.Output, target ports.Target) {
+	ctx, cancel := context.WithTimeout(ctx, flash.Limit)
+	defer cancel()
 	for _, o := range outputs {
 		if o.ID != target.OutputID {
 			continue
 		area := target.Region
 		if target.WorkspaceID != 0 {
 			area = recordedFrame(ctx, source, target)
+			if ctx.Err() != nil {
+				return // the budget went on the lookup
+			}
 		}
 		if err := flash.Show(ctx, o, area); err != nil {
 			log := logging.For(ctx, "app")

M internal/app/record_interactive.go +1 -1


@@ -188,7 +188,7 @@ func recordedFrame(ctx context.Context, source *wayland.Source, t ports.Target)
188	188		return ports.Region{}
189	189		}
190	190		log := logging.For(ctx, "app")
191		-	log.Warn().Err(err).Msg("workspace frame unavailable: HUD on the whole output")
	191	+	log.Warn().Err(err).Msg("workspace frame unavailable: using the whole output")
192	192		return ports.Region{}
193	193		}
194	194		for _, w := range list {


@@ -188,7 +188,7 @@ func recordedFrame(ctx context.Context, source *wayland.Source, t ports.Target)
188		return ports.Region{}	188		return ports.Region{}
189		}	189		}
190		log := logging.For(ctx, "app")	190		log := logging.For(ctx, "app")
191	-	log.Warn().Err(err).Msg("workspace frame unavailable: HUD on the whole output")	191	+	log.Warn().Err(err).Msg("workspace frame unavailable: using the whole output")
192		return ports.Region{}	192		return ports.Region{}
193		}	193		}
194		for _, w := range list {	194		for _, w := range list {