UnigramDev/Unigram/develop • 30ce540 • 5 files, +502/-195 Close the lock leak in the ClientService c

UnigramDev/Unigram/develop • 30ce540 • 5 files, +502/-195
Close the lock leak in the ClientService critical sections

Thirteen Monitor.Enter/Monitor.Exit pairs had no try/finally, so anything that
threw in between leaked the lock for good. The worst is UpdateChatLastMessage,
which constructs a MessageAlbumLastMessageService while holding the Chat: one
throw there locks that chat forever, the next UpdateChatPosition for it blocks
the TDLib receive thread, and since that thread is the only one draining
td_receive, every update in the app stops arriving with no crash to show for it.
GetChatFolders takes lock (chat) from the UI thread, so the chat list wedges too.

All thirteen are plain lock blocks now. The ten synchronous ones are a straight
wrap. The three paging methods could not be, because await is illegal inside
lock — which is exactly why they were written with a hand-placed Monitor.Exit
before the await in the first place. Each now decides under the lock how much is
still to be loaded and either builds its result and returns inside the lock, or
falls out of it and awaits with nothing held. Same semantics, same lock ordering,
one exit path instead of three.

The MOCKUP blocks in those three methods moved but were left as they are. MOCKUP
is not defined in any configuration and the blocks reference an undefined index
variable, so they have not compiled in a long time; quietly repairing dead code
that cannot be tested does not belong in a lock-safety change.

UpdateChatDraftMessage was not in the review that prompted this — it turned up
grepping for Monitor. after converting the twelve that were listed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

UnigramDev/Unigram/develop • 533cbd9 • 3 files, +335/-147
Close the lock leak in the two topic services

The same Monitor.Enter/Exit-without-try/finally as the ClientService partials,
in the six pairs the earlier pass did not reach: it only covered ClientService.*,
and these two services are separate files owned by it. Four in ForumTopicService,
two in DirectMessagesChatTopicService, and the freeze mode is the same — a throw
between a pair leaks _order for good, and the topic list for that chat never
sorts again.

LoadForumTopicsAsync was the interesting one. It called tsc.SetResult inside the
monitor, so the continuation waiting on it — GetForumTopicsAsyncImpl, which takes
_order itself — ran inline while the lock was held. Monitor being recursive is
the only reason that was a surprise rather than a deadlock, and a throw in there
would have skipped the Exit. It now builds the result under the lock and
completes the task after releasing it.

The two paging methods got the same restructure as the three before them: decide
under the lock how much is still to load, then either answer from the cache
inside it or fall out and await with nothing held. That is five copies of this
method carrying the same bug, which is a better argument for merging them than
the duplicated line count ever was.

UpdateTopicOrder keeps publishing outside the lock, where the hand-placed
Monitor.Exit already put it. The aggregator publish still inside the lock in
LoadForumTopicsAsync is a separate finding and is left alone here.

Also records the review of both services in clientservice-review.md: seventeen
findings, of which these six are the first fixed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

#unigram