small chat 3#29131
Conversation
| @@ -672,26 +672,45 @@ func (s *store) Add(ctx context.Context, convID chat1.ConversationID, | |||
| msgs []chat1.MessageUnboxed, | |||
| ) (err error) { | |||
| defer s.Trace(ctx, &err, "Add")() | |||
There was a problem hiding this comment.
Fixed two deadlocks in the chat search indexer that caused thread loading to break permanently after switching users.
Bug 1: Indexer.Clear held idx.Lock() while performing bulk LevelDB deletes (one per indexed message in a conversation). All GetThreadNonblock calls go through
Indexer.Suspend, which also needs idx.Lock(), so they blocked for the entire duration of the delete loop — potentially many seconds.
Bug 2: store.Add held the store's write mutex (s.Lock()) while calling ChatHelper.GetMessages to fetch superseded messages for EDIT and ATTACHMENTUPLOADED message types.
That call goes through ConvSource.GetMessages → ConversationLockTab, which has no timeout on the lock acquisition path when no deadlock cycle is detected. If any other
goroutine held the conversation lock at that moment, store.Add would block indefinitely while holding s.Lock(). Since Indexer.Clear calls store.ClearMemory (which also
needs s.Lock()), it too blocked indefinitely, keeping idx.Lock() held and freezing all thread loading.
The repro: send a message via CLI as a different user, then switch users. The background indexer queues work from the previous session; when it processes EDIT messages it
triggers the store.Add → conv lock path while Indexer.Clear is running for the new session, creating the deadlock.
Fixes:
- Removed the unnecessary idx.Lock() from Indexer.Clear — the store has its own concurrency controls and the indexer mutex guards unrelated fields (started, suspendCount).
- Moved the superseded-message fetch in store.Add to before s.Lock() is acquired, so the mutex is only held for pure in-memory index mutations.
cc: @zoom-ua
There was a problem hiding this comment.
please pick these into master cc @mmaxim
did you audit the search code for any similar issues? can you add a regression test too?
| @@ -672,26 +672,45 @@ func (s *store) Add(ctx context.Context, convID chat1.ConversationID, | |||
| msgs []chat1.MessageUnboxed, | |||
| ) (err error) { | |||
| defer s.Trace(ctx, &err, "Add")() | |||
There was a problem hiding this comment.
please pick these into master cc @mmaxim
did you audit the search code for any similar issues? can you add a regression test too?
| }, | ||
| onMessagesUpdated: messagesUpdated => { | ||
| if (!messagesUpdated.updates) return | ||
| const activelyLookingAtThread = Common.isUserActivelyLookingAtThisThread(get().id) |
There was a problem hiding this comment.
search indexing throws off a bunch of these as a side effect
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 42 out of 44 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Move more out of inbox.
Unify how search bar works
Remove defer into component itself and simplify
Fix highlight and enter not working on inbox items sometimes