From 952d3c82049a5521c506d7e30e6118361f2c7f4d Mon Sep 17 00:00:00 2001 From: Angus Kola Date: Fri, 6 Nov 2026 19:01:25 -0500 Subject: [PATCH] Fix Remote Braille ordering and lease cleanup --- internal/app/braille_frames.go | 4 + internal/app/braille_remote.go | 14 +-- internal/app/tentacle_remote_braille.go | 21 +++-- internal/app/tentacle_remote_braille_test.go | 98 ++++++++++++++++++++ 4 files changed, 123 insertions(+), 14 deletions(-) diff --git a/internal/app/braille_frames.go b/internal/app/braille_frames.go index 0078fbc..afe7c87 100644 --- a/internal/app/braille_frames.go +++ b/internal/app/braille_frames.go @@ -142,8 +142,12 @@ func (worker *brailleWorker) enqueueSecureFrameLocked(frame braille.Frame, base // Final peer cells: never translate or wrap them again. frame.Contract, frame.Table, frame.ExpandAtCursor, frame.WordWrap = false, "", false, false } + restoresBase := frame.ID != 0 && frame.ID == worker.baseFrame.ID worker.sequence++ frame.ID = worker.sequence + if base || restoresBase { + worker.baseFrame.ID = frame.ID + } if base { worker.export_locked(frame, frame.ID) } diff --git a/internal/app/braille_remote.go b/internal/app/braille_remote.go index 59ca745..a14444c 100644 --- a/internal/app/braille_remote.go +++ b/internal/app/braille_remote.go @@ -37,31 +37,31 @@ func (worker *brailleWorker) export_locked(frame braille.Frame, id uint64) { worker.remoteExport(frame) } -// SetRemoteExport installs or removes (nil) the exporter and returns the -// current application frame so a new export starts from it. -func (worker *brailleWorker) SetRemoteExport(export func(braille.Frame)) (braille.Frame, bool) { +// SetRemoteExport installs or removes (nil) the exporter. The initial frame +// and subsequent updates are offered under mu, preserving their order. +func (worker *brailleWorker) SetRemoteExport(export func(braille.Frame)) { worker.mu.Lock() defer worker.mu.Unlock() worker.remoteExport = export if export == nil || worker.secure { - return braille.Frame{}, false + return } frame := worker.baseFrame frame.Contract, frame.Table, frame.WordWrap = worker.contracted, worker.table, worker.wordWrap - if worker.internalUIFrame == nil && worker.remoteFrame == nil { + if worker.internalUIFrame == nil && worker.remoteFrame == nil && worker.frame.ID == frame.ID { frame.ID = worker.frame.ID } else { frame.ID = 0 } frame.Routes, frame.Virtual, frame.Document = nil, nil, nil frame.Dots = append([]byte(nil), frame.Dots...) - return frame, true + export(frame) } // RouteExported routes a cell chosen on a peer's display. The event must name // the frame currently shown here; anything else is stale and ignored. func (worker *brailleWorker) RouteExported(ctx context.Context, event braille.RoutingEvent) { - if event.ID == 0 || event.Pan != "" { + if ctx.Err() != nil || event.ID == 0 || event.Pan != "" { return } worker.route(ctx, event) diff --git a/internal/app/tentacle_remote_braille.go b/internal/app/tentacle_remote_braille.go index 3c28185..80f9e5b 100644 --- a/internal/app/tentacle_remote_braille.go +++ b/internal/app/tentacle_remote_braille.go @@ -9,7 +9,7 @@ import ( "context" "encoding/json" "errors" - "io" + "os" "os/exec" "slices" "time" @@ -29,7 +29,7 @@ type virtualBraille interface { type processVirtualBraille struct { cmd *exec.Cmd - stdin io.WriteCloser + stdin *os.File encoder *json.Encoder responses chan braille.VirtualResponse done chan struct{} @@ -45,12 +45,13 @@ func start_virtual_braille(path string) (virtualBraille, error) { } stdout, err := cmd.StdoutPipe() if err != nil { + _ = stdin.Close() return nil, err } if err := cmd.Start(); err != nil { return nil, err } - helper := &processVirtualBraille{cmd: cmd, stdin: stdin, encoder: json.NewEncoder(stdin), + helper := &processVirtualBraille{cmd: cmd, stdin: stdin.(*os.File), encoder: json.NewEncoder(stdin), responses: make(chan braille.VirtualResponse, 16), done: make(chan struct{})} go func() { defer close(helper.responses) @@ -72,6 +73,10 @@ func start_virtual_braille(path string) (virtualBraille, error) { } func (helper *processVirtualBraille) Send(request braille.VirtualRequest) error { + // A wedged translator must not prevent lease cleanup or owner shutdown. + if err := helper.stdin.SetWriteDeadline(time.Now().Add(time.Second)); err != nil { + return err + } return helper.encoder.Encode(request) } @@ -197,9 +202,7 @@ func (runtime *tentacleRuntime) start_remote_braille(ctx context.Context, sessio continue } helper, responses, exportLease, exportDone = started, started.Responses(), delivery.Lease, delivery.Context.Done() - if frame, ok := worker.SetRemoteExport(offer); ok { - offer(frame) - } + worker.SetRemoteExport(offer) } width := value.Width if helper.Send(braille.VirtualRequest{Width: &width}) != nil { @@ -240,7 +243,11 @@ func (runtime *tentacleRuntime) start_remote_braille(ctx context.Context, sessio continue } if response.Route != nil { - worker.RouteExported(ctx, *response.Route) + // A buffered helper response can race lease revocation. + leaseCtx, err := session.RemoteContext(exportLease, "braille-stream") + if err == nil && allowed() { + worker.RouteExported(leaseCtx, *response.Route) + } } else if len(response.Cells) != 0 && allowed() { if errors.Is(session.PublishRemoteBraille(exportLease, tentaclewire.RemoteBraille{Op: "cells", Cells: response.Cells}), tentacle.ErrLimit) { stopExport() diff --git a/internal/app/tentacle_remote_braille_test.go b/internal/app/tentacle_remote_braille_test.go index 27b8696..792602c 100644 --- a/internal/app/tentacle_remote_braille_test.go +++ b/internal/app/tentacle_remote_braille_test.go @@ -7,14 +7,18 @@ package app import ( "context" "encoding/json" + "errors" "log/slog" + "os" "reflect" + "strings" "sync" "testing" "time" "git.stormux.org/storm/cthulhu/internal/atspi" "git.stormux.org/storm/cthulhu/internal/braille" + "git.stormux.org/storm/cthulhu/internal/config" "git.stormux.org/storm/cthulhu/internal/tentacle" "git.stormux.org/storm/cthulhu/internal/tentaclewire" ) @@ -200,3 +204,97 @@ func TestRemoteBrailleControllerShowsPeerCellsAndReportsKeys(t *testing.T) { } policy.SetAvailable(true) } + +func TestRemoteBrailleInitialExportDuringFlashIsUnroutable(t *testing.T) { + worker := &brailleWorker{cancel: func() {}, connected: true, wake: make(chan struct{}, 1)} + worker.EnqueueFrame(braille.Frame{Text: "application", Cursor: -1}) + baseID := worker.current().ID + var exported braille.Frame + worker.SetRemoteExport(func(frame braille.Frame) { exported = frame }) + if exported.ID != baseID { + t.Fatal("initial application lost its routing identity") + } + worker.FlashConfirmation("message", config.BrailleOptions{FlashMessages: true, FlashPersistent: true}) + worker.SetRemoteExport(func(frame braille.Frame) { exported = frame }) + if exported.Text != "application" || exported.ID != 0 { + t.Fatalf("flash export = %+v", exported) + } + worker.DismissConfirmation() + worker.SetRemoteExport(func(frame braille.Frame) { exported = frame }) + if exported.ID == 0 || exported.ID != worker.current().ID { + t.Fatal("restored application lost its routing identity") + } +} + +func TestRemoteBrailleInitialExportSerializedWithUpdates(t *testing.T) { + worker := &brailleWorker{cancel: func() {}, wake: make(chan struct{}, 1)} + worker.EnqueueFrame(braille.Frame{Text: "initial", Cursor: -1}) + entered, release, finished := make(chan struct{}), make(chan struct{}), make(chan struct{}) + var frames []string + go func() { + worker.SetRemoteExport(func(frame braille.Frame) { + if frame.Text == "initial" { + close(entered) + <-release + } + frames = append(frames, frame.Text) + }) + close(finished) + }() + select { + case <-entered: + case <-time.After(5 * time.Second): + close(release) + t.Fatal("initial export was not offered") + } + if worker.mu.TryLock() { + worker.mu.Unlock() + close(release) + <-finished + t.Fatal("initial export was offered outside the frame lock") + } + updated := make(chan struct{}) + go func() { worker.EnqueueFrame(braille.Frame{Text: "newer", Cursor: -1}); close(updated) }() + close(release) + <-finished + <-updated + if !reflect.DeepEqual(frames, []string{"initial", "newer"}) { + t.Fatal(frames) + } +} + +func TestRemoteBrailleCancelledRouteHasNoEffect(t *testing.T) { + router := peerRouteRecorder{routes: make(chan atspi.BrailleRoute, 1)} + worker := &brailleWorker{cancel: func() {}, wake: make(chan struct{}, 1), router: router} + worker.EnqueueFrame(braille.Frame{Text: "test", Cursor: -1, Routes: []braille.TextRoute{{Start: 0, End: 4, TextLength: 4}}}) + ctx, cancel := context.WithCancel(t.Context()) + cancel() + worker.RouteExported(ctx, braille.RoutingEvent{ID: worker.current().ID, Offset: 1}) + select { + case <-router.routes: + t.Fatal("cancelled route reached application") + default: + } +} + +func TestRemoteBrailleWedgedHelperWriteIsBounded(t *testing.T) { + reader, writer, err := os.Pipe() + if err != nil { + t.Fatal(err) + } + defer reader.Close() + defer writer.Close() + helper := &processVirtualBraille{stdin: writer, encoder: json.NewEncoder(writer)} + done := make(chan error, 1) + go func() { + done <- helper.Send(braille.VirtualRequest{Frame: &braille.Frame{Text: strings.Repeat("x", 1024*1024)}}) + }() + select { + case err := <-done: + if !errors.Is(err, os.ErrDeadlineExceeded) { + t.Fatalf("blocked write = %v", err) + } + case <-time.After(5 * time.Second): + t.Fatal("wedged helper blocked its owner") + } +}