UnigramDev/Unigram/composer-typing-performance • d7b1449 • 1 files, +31/-8
Reconcile the review checkboxes with what actually landed
The E2E delegate signature item was still marked open although the byte[] work
closed it, and the transform semaphore it exposed was not written down at all.
The five questions Fela ruled on are marked decided rather than outstanding, and
mono screencast audio as deferred upstream.
Two genuinely open items remain: m_impl unguarded in VoipGroupManager, and the
unmeasured buffer between the WASAPI and WebRTC clocks.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
UnigramDev/Unigram/composer-typing-performance • e8aa325 • 1 files, +41/-8
Re-aim the NewDictionary finding at Clear()
The item claimed the mutating indexer getter, plus _haveFullChatList being set
outside the monitor, amounted to a cross-thread race. It does not. All seven
callers of GetChatListAsync and GetStoryListAsync are UI-layer and the repo has
no ConfigureAwait(false), so those continuations resume on the UI thread, and
every use of the mutating getter is already inside a lock. No second thread reads
those dictionaries under the monitor at all. Demoted to P3 as a trap for the next
reader rather than a defect.
The real cross-thread party is Clear(), which runs on the TDLib receive thread
and takes two locks while emptying eight collections that every other accessor
guards — _chatList and _haveFullChatList, _storyList and _haveFullStoryList,
_savedMessages, _savedMessagesTags, _suggestedActions and _chatFolders2. A logout
arriving while the UI has a chat list load in flight can clear a SortedSet out
from under an enumerator.
Which the earlier Clear() fix missed: it audited which fields were cleared and
never asked whether clearing them was synchronised.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
UnigramDev/Unigram/composer-typing-performance • 1632e05 • 2 files, +47/-14
Hold the locks while Clear() empties the collections they guard
Clear() runs on the TDLib receive thread, from OnResult on
AuthorizationStateClosed, and was taking two locks while emptying nine
collections that every other accessor guards. A logout landing while the UI had a
chat list load in flight could clear a SortedSet out from under an enumerator, or
a Dictionary mid-lookup.
Each group now takes the same lock its readers do: _chatList with
_haveFullChatList, _storyList with _haveFullStoryList, _savedMessages,
_savedMessagesTags, _suggestedActions, _chatFolders with _chatFolders2, and
_timezones. They are taken one after another and never nested, so no lock
ordering is introduced.
_timezones was not in the list this started from. Enumerating every lock target
across the partials found it, which is the check worth repeating when a field is
added — the same lesson as the coverage audit, one level down.
This is what the earlier Clear() fix missed: it audited which fields were
cleared, and never asked whether clearing them was synchronised.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
#unigram
Post #20593
57