UnigramDev/Unigram/composer-typing-performance • bdf4304 • 3 files, +50/-13
Make the three loose dictionaries thread-safe, mostly by type
_chatAccessibleUntil, _cachedReactions and _preparedLogsFileIds were plain
collections shared between the UI thread and the TDLib one. The first two are
written and read from the UI and emptied by Clear() on the receive thread; the
third is worse, and not merely latent: PrepareLogs does ??= new() then Add() on
the UI thread while UpdateFile assigns the field null on the TDLib thread, so a
call landing between those two statements dereferences null.
Fela's point on approach is right, so the rule is now explicit rather than picked
per site. A standalone dictionary whose operations are single calls becomes a
ReaderWriterDictionary, like the rest of the caches. A lock is only for what that
type cannot express: a set rather than a dictionary, a field assigned null, or an
operation that has to be compound.
So _chatAccessibleUntil and _cachedReactions are ReaderWriterDictionary now and
carry no explicit locking at all — the call sites got shorter rather than longer.
_preparedLogsFileIds fails all three tests, so it keeps a lock and says why where
it is declared. Its remove-then-maybe-reset is one critical section now, with the
Client.Execute that restores the verbosity hoisted out of it.
That same rule is what put ForumTopicService on a single lock earlier: six of its
eight collections are sets, a list and a sorted set, with compounds spanning
them, so the type would have covered two of eight and left two lock domains.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
UnigramDev/Unigram/composer-typing-performance • 43afd24 • 1 files, +18/-2
Close D3: the _haveFullList write races with nothing
The item said it was the same shape as the _haveFullChatList finding above it,
and it was — including being wrong for the same reason, so it inherits the
correction rather than the bug.
_haveFullList is a plain bool with three references: the declaration, one read
inside the lock and one write outside it. Its only caller is TopicListViewModel,
and with no ConfigureAwait(false) anywhere in the repo that continuation resumes
on the UI thread, the same one that took the lock. The TDLib thread reaches this
class only through UpdateDirectMessagesChatTopic, which never touches the field.
Granting a second thread anyway, a bool write is atomic, so the worst case is one
redundant loadDirectMessagesChatTopics rather than corruption.
The same reasoning clears ForumTopicService._haveFullList, which has the
identical shape and was never raised. In both, the await completes on a
TaskCompletionSource set from the TDLib thread, but the continuation posts back
to the UI context — which is what makes the lock-then-unlocked-write sequence
single-threaded in the first place.
No code change.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
#unigram
Post #20594
45