92bee5e fix(systemviews): harden action lifecycle · dumber

M assets/systemviews/systemviews.wasm.br +0 -0 binary file: no text diff

binary file: no text diff

M internal/infrastructure/systemviewsbridge/browser_transport_js.go +44 -8


@@ -249,24 +249,52 @@ func awaitPromise(ctx context.Context, promise js.Value) (js.Value, error) {
249	249		}
250	250		resultCh := make(chan result, 1)
251	251
252		-	thenFn := js.FuncOf(func(_ js.Value, args []js.Value) any {
	252	+	var (
	253	+	callbackMu sync.Mutex
	254	+	active = true
	255	+	released bool
	256	+	thenFn js.Func
	257	+	catchFn js.Func
	258	+	)
	259	+	releaseCallbacksLocked := func() {
	260	+	if released {
	261	+	return
	262	+	}
	263	+	released = true
	264	+	thenFn.Release()
	265	+	catchFn.Release()
	266	+	}
	267	+	publish := func(res result) {
	268	+	callbackMu.Lock()
	269	+	defer callbackMu.Unlock()
	270	+	if !active {
	271	+	releaseCallbacksLocked()
	272	+	return
	273	+	}
	274	+	active = false
	275	+	select {
	276	+	case resultCh <- res:
	277	+	default:
	278	+	}
	279	+	releaseCallbacksLocked()
	280	+	}
	281	+
	282	+	thenFn = js.FuncOf(func(_ js.Value, args []js.Value) any {
253	283		if len(args) == 0 {
254		-	resultCh <- result{value: js.Undefined()}
	284	+	publish(result{value: js.Undefined()})
255	285		return nil
256	286		}
257		-	resultCh <- result{value: args[0]}
	287	+	publish(result{value: args[0]})
258	288		return nil
259	289		})
260		-	catchFn := js.FuncOf(func(_ js.Value, args []js.Value) any {
	290	+	catchFn = js.FuncOf(func(_ js.Value, args []js.Value) any {
261	291		if len(args) == 0 {
262		-	resultCh <- result{err: errors.New("promise rejected")}
	292	+	publish(result{err: errors.New("promise rejected")})
263	293		return nil
264	294		}
265		-	resultCh <- result{err: errors.New(args[0].String())}
	295	+	publish(result{err: errors.New(args[0].String())})
266	296		return nil
267	297		})
268		-	defer thenFn.Release()
269		-	defer catchFn.Release()
270	298
271	299		promise.Call("then", thenFn).Call("catch", catchFn)
272	300

@@ -274,6 +302,14 @@ func awaitPromise(ctx context.Context, promise js.Value) (js.Value, error) {
274	302		case res := <-resultCh:
275	303		return res.value, res.err
276	304		case <-ctx.Done():
	305	+	select {
	306	+	case res := <-resultCh:
	307	+	return res.value, res.err
	308	+	default:
	309	+	}
	310	+	callbackMu.Lock()
	311	+	active = false
	312	+	callbackMu.Unlock()
277	313		return js.Undefined(), ctx.Err()
278	314		}
279	315		}


@@ -249,24 +249,52 @@ func awaitPromise(ctx context.Context, promise js.Value) (js.Value, error) {
249		}	249		}
250		resultCh := make(chan result, 1)	250		resultCh := make(chan result, 1)
251			251
252	-	thenFn := js.FuncOf(func(_ js.Value, args []js.Value) any {	252	+	var (
			253	+	callbackMu sync.Mutex
			254	+	active = true
			255	+	released bool
			256	+	thenFn js.Func
			257	+	catchFn js.Func
			258	+	)
			259	+	releaseCallbacksLocked := func() {
			260	+	if released {
			261	+	return
			262	+	}
			263	+	released = true
			264	+	thenFn.Release()
			265	+	catchFn.Release()
			266	+	}
			267	+	publish := func(res result) {
			268	+	callbackMu.Lock()
			269	+	defer callbackMu.Unlock()
			270	+	if !active {
			271	+	releaseCallbacksLocked()
			272	+	return
			273	+	}
			274	+	active = false
			275	+	select {
			276	+	case resultCh <- res:
			277	+	default:
			278	+	}
			279	+	releaseCallbacksLocked()
			280	+	}
			281	+
			282	+	thenFn = js.FuncOf(func(_ js.Value, args []js.Value) any {
253		if len(args) == 0 {	283		if len(args) == 0 {
254	-	resultCh <- result{value: js.Undefined()}	284	+	publish(result{value: js.Undefined()})
255		return nil	285		return nil
256		}	286		}
257	-	resultCh <- result{value: args[0]}	287	+	publish(result{value: args[0]})
258		return nil	288		return nil
259		})	289		})
260	-	catchFn := js.FuncOf(func(_ js.Value, args []js.Value) any {	290	+	catchFn = js.FuncOf(func(_ js.Value, args []js.Value) any {
261		if len(args) == 0 {	291		if len(args) == 0 {
262	-	resultCh <- result{err: errors.New("promise rejected")}	292	+	publish(result{err: errors.New("promise rejected")})
263		return nil	293		return nil
264		}	294		}
265	-	resultCh <- result{err: errors.New(args[0].String())}	295	+	publish(result{err: errors.New(args[0].String())})
266		return nil	296		return nil
267		})	297		})
268	-	defer thenFn.Release()
269	-	defer catchFn.Release()
270			298
271		promise.Call("then", thenFn).Call("catch", catchFn)	299		promise.Call("then", thenFn).Call("catch", catchFn)
272			300

@@ -274,6 +302,14 @@ func awaitPromise(ctx context.Context, promise js.Value) (js.Value, error) {
274		case res := <-resultCh:	302		case res := <-resultCh:
275		return res.value, res.err	303		return res.value, res.err
276		case <-ctx.Done():	304		case <-ctx.Done():
			305	+	select {
			306	+	case res := <-resultCh:
			307	+	return res.value, res.err
			308	+	default:
			309	+	}
			310	+	callbackMu.Lock()
			311	+	active = false
			312	+	callbackMu.Unlock()
277		return js.Undefined(), ctx.Err()	313		return js.Undefined(), ctx.Err()
278		}	314		}
279		}	315		}

M internal/ui/systemviews/app.go +30 -9


@@ -21,6 +21,7 @@ type Dependencies struct {
21	21
22	22		type App struct {
23	23		mu sync.Mutex
	24	+	actionMu sync.Mutex
24	25		deps Dependencies
25	26		currentRoute Route
26	27		shellTheme shellTheme

@@ -85,6 +86,7 @@ func (a *App) RunWithContext(ctx context.Context) error {
85	86		}
86	87
87	88		func (a *App) bindDOMActions(ctx context.Context, binder DOMActionBinder) error {
	89	+	a.actionMu.Lock()
88	90		if a.actionQueue == nil {
89	91		workerCtx, cancel := context.WithCancel(ctx)
90	92		a.actionQueue = make(chan DOMAction, 64)

@@ -92,24 +94,40 @@ func (a *App) bindDOMActions(ctx context.Context, binder DOMActionBinder) error
92	94		a.actionCancel = cancel
93	95		go a.runActionWorker(workerCtx)
94	96		}
	97	+	a.actionMu.Unlock()
95	98		return binder.BindActions(func(action DOMAction) {
96		-	if !a.enqueueDOMAction(action) {
	99	+	if !a.enqueueDOMAction(action) && a.actionWorkerActive() {
97	100		go a.surfaceActionError(fmt.Errorf("systemview is busy; dropped action %q", action.Action))
98	101		}
99	102		})
100	103		}
101	104
	105	+	func (a *App) actionWorkerActive() bool {
	106	+	if a == nil {
	107	+	return false
	108	+	}
	109	+	a.actionMu.Lock()
	110	+	queue := a.actionQueue
	111	+	ctx := a.actionCtx
	112	+	a.actionMu.Unlock()
	113	+	return queue != nil && ctx != nil && ctx.Err() == nil
	114	+	}
	115	+
102	116		func (a *App) enqueueDOMAction(action DOMAction) bool {
103		-	if a == nil \|\| a.actionQueue == nil {
	117	+	if a == nil {
104	118		return false
105	119		}
106		-	if a.actionCtx == nil {
	120	+	a.actionMu.Lock()
	121	+	queue := a.actionQueue
	122	+	ctx := a.actionCtx
	123	+	a.actionMu.Unlock()
	124	+	if queue == nil \|\| ctx == nil {
107	125		return false
108	126		}
109	127		select {
110		-	case <-a.actionCtx.Done():
	128	+	case <-ctx.Done():
111	129		return false
112		-	case a.actionQueue <- action:
	130	+	case queue <- action:
113	131		return true
114	132		default:
115	133		return false

@@ -146,12 +164,15 @@ func (a *App) Close() {
146	164		if a == nil {
147	165		return
148	166		}
149		-	if a.actionCancel != nil {
150		-	a.actionCancel()
151		-	a.actionCancel = nil
152		-	}
	167	+	a.actionMu.Lock()
	168	+	cancel := a.actionCancel
	169	+	a.actionCancel = nil
153	170		a.actionCtx = nil
154	171		a.actionQueue = nil
	172	+	a.actionMu.Unlock()
	173	+	if cancel != nil {
	174	+	cancel()
	175	+	}
155	176		if releaser, ok := a.deps.DOM.(interface{ Release() }); ok {
156	177		releaser.Release()
157	178		}

 type App struct {
 	mu                   sync.Mutex
+	actionMu             sync.Mutex
 	deps                 Dependencies
 	currentRoute         Route
 	shellTheme           shellTheme
 }
 func (a *App) bindDOMActions(ctx context.Context, binder DOMActionBinder) error {
+	a.actionMu.Lock()
 	if a.actionQueue == nil {
 		workerCtx, cancel := context.WithCancel(ctx)
 		a.actionQueue = make(chan DOMAction, 64)
 		a.actionCancel = cancel
 		go a.runActionWorker(workerCtx)
 	}
+	a.actionMu.Unlock()
 	return binder.BindActions(func(action DOMAction) {
-		if !a.enqueueDOMAction(action) {
+		if !a.enqueueDOMAction(action) && a.actionWorkerActive() {
 			go a.surfaceActionError(fmt.Errorf("systemview is busy; dropped action %q", action.Action))
 		}
 	})
 }
+func (a *App) actionWorkerActive() bool {
+	if a == nil {
+		return false
+	}
+	a.actionMu.Lock()
+	queue := a.actionQueue
+	ctx := a.actionCtx
+	a.actionMu.Unlock()
+	return queue != nil && ctx != nil && ctx.Err() == nil
+}
 func (a *App) enqueueDOMAction(action DOMAction) bool {
-	if a == nil || a.actionQueue == nil {
+	if a == nil {
 		return false
 	}
-	if a.actionCtx == nil {
+	a.actionMu.Lock()
+	queue := a.actionQueue
+	ctx := a.actionCtx
+	a.actionMu.Unlock()
+	if queue == nil || ctx == nil {
 		return false
 	}
 	select {
-	case <-a.actionCtx.Done():
+	case <-ctx.Done():
 		return false
-	case a.actionQueue <- action:
+	case queue <- action:
 		return true
 	default:
 		return false
 	if a == nil {
 		return
 	}
-	if a.actionCancel != nil {
+	a.actionMu.Lock()
-		a.actionCancel()
+	cancel := a.actionCancel
-		a.actionCancel = nil
+	a.actionCancel = nil
-	}
 	a.actionCtx = nil
 	a.actionQueue = nil
+	a.actionMu.Unlock()
+	if cancel != nil {
+		cancel()
+	}
 	if releaser, ok := a.deps.DOM.(interface{ Release() }); ok {
 		releaser.Release()
 	}

M internal/ui/systemviews/app_test.go +33 -0


@@ -163,6 +163,24 @@ func TestAppHandleHistoryActionsRefreshesDOM(t *testing.T) {
163	163		assert.Contains(t, dom.html, "Deleted history for example.com")
164	164		}
165	165
	166	+	func TestAppCloseStopsActionWorkerAndReleasesDOM(t *testing.T) {
	167	+	dom := &fakeActionDOM{}
	168	+	history := &fakeHistoryService{entries: []*entity.HistoryEntry{{ID: 1, URL: "https://example.com"}}}
	169	+	app := NewApp(Dependencies{DOM: dom, History: history, LocationURI: "dumb://history"})
	170	+
	171	+	ctx, cancel := context.WithCancel(context.Background())
	172	+	defer cancel()
	173	+	require.NoError(t, app.RunWithContext(ctx))
	174	+	require.NotNil(t, dom.handler)
	175	+
	176	+	app.Close()
	177	+	assert.True(t, dom.released)
	178	+	assert.False(t, app.enqueueDOMAction(DOMAction{Action: historyActionClear}))
	179	+	require.NotPanics(t, func() {
	180	+	dom.handler(DOMAction{Action: historyActionClear})
	181	+	})
	182	+	}
	183	+
166	184		func TestAppLoadInitialHistoryRouteRendersErrorState(t *testing.T) {
167	185		t.Parallel()
168	186

@@ -627,6 +645,21 @@ func (d *fakeDOM) Mount(html string) error {
627	645		return nil
628	646		}
629	647
	648	+	type fakeActionDOM struct {
	649	+	fakeDOM
	650	+	handler DOMActionHandler
	651	+	released bool
	652	+	}
	653	+
	654	+	func (d *fakeActionDOM) BindActions(handler DOMActionHandler) error {
	655	+	d.handler = handler
	656	+	return nil
	657	+	}
	658	+
	659	+	func (d *fakeActionDOM) Release() {
	660	+	d.released = true
	661	+	}
	662	+
630	663		// Handwritten fake to capture history state for stateful render assertions.
631	664		type fakeHistoryService struct {
632	665		called bool


@@ -163,6 +163,24 @@ func TestAppHandleHistoryActionsRefreshesDOM(t *testing.T) {
163	assert.Contains(t, dom.html, "Deleted history for example.com")	163		assert.Contains(t, dom.html, "Deleted history for example.com")
164	}	164		}
165		165
		166	+	func TestAppCloseStopsActionWorkerAndReleasesDOM(t *testing.T) {
		167	+	dom := &fakeActionDOM{}
		168	+	history := &fakeHistoryService{entries: []*entity.HistoryEntry{{ID: 1, URL: "https://example.com"}}}
		169	+	app := NewApp(Dependencies{DOM: dom, History: history, LocationURI: "dumb://history"})
		170	+
		171	+	ctx, cancel := context.WithCancel(context.Background())
		172	+	defer cancel()
		173	+	require.NoError(t, app.RunWithContext(ctx))
		174	+	require.NotNil(t, dom.handler)
		175	+
		176	+	app.Close()
		177	+	assert.True(t, dom.released)
		178	+	assert.False(t, app.enqueueDOMAction(DOMAction{Action: historyActionClear}))
		179	+	require.NotPanics(t, func() {
		180	+	dom.handler(DOMAction{Action: historyActionClear})
		181	+	})
		182	+	}
		183	+
166	func TestAppLoadInitialHistoryRouteRendersErrorState(t *testing.T) {	184		func TestAppLoadInitialHistoryRouteRendersErrorState(t *testing.T) {
167	t.Parallel()	185		t.Parallel()
168		186

@@ -627,6 +645,21 @@ func (d *fakeDOM) Mount(html string) error {
627	return nil	645		return nil
628	}	646		}
629		647
		648	+	type fakeActionDOM struct {
		649	+	fakeDOM
		650	+	handler DOMActionHandler
		651	+	released bool
		652	+	}
		653	+
		654	+	func (d *fakeActionDOM) BindActions(handler DOMActionHandler) error {
		655	+	d.handler = handler
		656	+	return nil
		657	+	}
		658	+
		659	+	func (d *fakeActionDOM) Release() {
		660	+	d.released = true
		661	+	}
		662	+
630	// Handwritten fake to capture history state for stateful render assertions.	663		// Handwritten fake to capture history state for stateful render assertions.
631	type fakeHistoryService struct {	664		type fakeHistoryService struct {
632	called bool	665		called bool