From e53c72be52277509957070a27a7051719ed685c2 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Mon, 11 May 2026 17:46:57 +0200 Subject: [PATCH] Make ViewBufferManager.NewTask respect call order NewTask was incrementing newTaskID and reading taskID inside the spawned goroutine, so for two NewTask calls in quick succession the assignment was determined by goroutine scheduling order rather than call order. When the goroutines reordered, the first NewTask call could end up with the higher taskID and "win" the staleness check, superseding the second call's task even though the caller intended the second to be the latest. Worse, the staleness check ran after onNewKey, so a goroutine destined to bail as stale would still reset the view buffer first, potentially wiping the winning task's already-written output. Take newTaskID++ synchronously in NewTask so taskIDs follow call order, and move the first staleness check ahead of onNewKey so a stale task doesn't side-effect the view before exiting. Co-Authored-By: Claude Opus 4.7 (1M context) --- pkg/tasks/tasks.go | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/pkg/tasks/tasks.go b/pkg/tasks/tasks.go index f534d01b4..7fadbb451 100644 --- a/pkg/tasks/tasks.go +++ b/pkg/tasks/tasks.go @@ -403,12 +403,29 @@ func (self *ViewBufferManager) NewTask(f func(TaskOpts) error, key string) error }) } + // Assign the taskID synchronously so it reflects NewTask call order + // rather than the order in which the spawned goroutines happen to be + // scheduled. Otherwise two NewTask calls in quick succession can have + // their goroutines race, with the later-called task ending up with the + // lower taskID and losing the staleness check below. + self.taskIDMutex.Lock() + self.newTaskID++ + taskID := self.newTaskID + self.taskIDMutex.Unlock() + go utils.Safe(func() { defer completeGocuiTask() self.taskIDMutex.Lock() - self.newTaskID++ - taskID := self.newTaskID + + // Bail out before touching shared view state if a newer task has + // already been queued: if we ran onNewKey here we'd reset the view + // for a task that's about to exit, potentially wiping output the + // winning task has already written. + if taskID < self.newTaskID { + self.taskIDMutex.Unlock() + return + } if self.GetTaskKey() != key && self.onNewKey != nil { self.onNewKey() @@ -419,6 +436,8 @@ func (self *ViewBufferManager) NewTask(f func(TaskOpts) error, key string) error self.waitingMutex.Lock() + // Re-check staleness after acquiring waitingMutex: a newer task + // may have arrived while we were blocked here. self.taskIDMutex.Lock() if taskID < self.newTaskID { self.waitingMutex.Unlock()