Repository navigation
Write and send emoji reactions off the main thread - #2245
Draft
mpretty-cyro wants to merge 2 commits into
Draft
mpretty-cyro wants to merge 2 commits into
mpretty-cyro wants to merge 2 commits into
Conversation
Picking a reaction wrote it to ReactionDatabase in a transaction, looked up the community server ID and persisted the send job, all on the main thread from the reaction overlay's touch handler. On a busy database that froze the conversation until the app was killed for not responding. Removing a reaction did the same. Both now run in the app-wide scope on an IO thread, then reload the conversation.
Each reaction ran in its own IO coroutine, so a removal could be sent before the add it undoes. Run them on one shared lane, and look up the existing reaction for "react with any" on that lane too, so it sees a reaction still being written.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Contributor checklist
playflavour,debugFixes #1234syntaxDescription
ANR sample: main thread in
ConversationReactionOverlay.handleUpEvent→ConversationActivityV2.sendEmojiReaction→ReactionDatabase.addReaction→beginTransaction, waiting for a connection.Adding and removing a reaction now do their database work and send in the app-wide scope on
Dispatchers.IO, then reload theconversation from
lifecycleScope. The rate limiter and UI checks stay on the main thread.They run one at a time on a shared lane (
Dispatchers.IO.limitedParallelism(1)), so a removal can't be sent before the add it undoes. "React with any" looks up your existing reaction on that lane too, so it sees one that is still being written instead of adding the same emoji twice.Testing: reproduced on an emulator with a debug-only hook (not in this PR) that holds the database write lock: picking a reaction ANR'd on
devwith the main thread insendEmojiReaction→ReactionDatabase.addReactions. With this change: no ANR, and the reaction was saved once the lock was released. Removing a reaction still works. The ordering on the shared lane is by construction; a removal overtaking its add wasn't reproduced ondev.