Skip to content

fix: Telegram session creation holds the global session mutex while doing blocking DB writes#894

Open
sam-saffron-jarvis wants to merge 1 commit into
SamSaffron:mainfrom
sam-saffron-jarvis:feat/codereview-93d1da64
Open

fix: Telegram session creation holds the global session mutex while doing blocking DB writes#894
sam-saffron-jarvis wants to merge 1 commit into
SamSaffron:mainfrom
sam-saffron-jarvis:feat/codereview-93d1da64

Conversation

@sam-saffron-jarvis

Copy link
Copy Markdown
Contributor

What changed

  • Publish the winning Telegram session under telegramSessionMgr.mu, then release the global session mutex before writing session metadata to the store.
  • Give session metadata persistence its own 5s context.WithTimeout(context.WithoutCancel(ctx), ...) so a canceled or unbounded serve context cannot leave Create/SetCurrent wedged forever.
  • Add regression coverage for a blocked Create call: another chat can still create and return its session while the first persistence call is stalled.
  • Add coverage that persistSession passes a bounded, non-canceled context to store writes.

Why this is high-value

Sam uses the Telegram/Jarvis path daily. Before this change, a slow or locked session store write during new/reset session persistence held the single telegramSessionMgr.mu, blocking every other chat's getOrCreate, reset, and shutdown session-map access. Moving only the blocking I/O out of the mutex preserves the winner decision while preventing one SQLite/filesystem stall from making the Telegram bot globally appear stuck.

Validation

  • gofmt -w internal/serve/telegram.go internal/serve/telegram_test.go
  • go build ./...
  • go test ./internal/serve -run 'TestTelegramSessionMgr(GetOrCreate_DoesNotBlockOtherChatsWhilePersistingSession|PersistSession_UsesBoundedContextDetachedFromCallerCancel|GetOrCreate_PersistsOnlyWinningConcurrentSession|ResetSessionIfCurrent_RejectsStaleExpectedSession)'
  • git diff --check
  • go test ./... was run twice; both runs reached the changed internal/serve tests successfully but failed in unchanged internal/jobs.TestProgramRunnerDoesNotLeakBackgroundChildren because its background child process was still observed alive after 1s.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant