Fix: GroupUpdated send jobs could not be read back after a restart - #2211
Draft
mpretty-cyro wants to merge 1 commit into
Draft
Fix: GroupUpdated send jobs could not be read back after a restart#2211mpretty-cyro wants to merge 1 commit into
mpretty-cyro wants to merge 1 commit into
Conversation
MessageSendJob persisted its message with a bare Kryo, whose reflective field serializer cannot construct a protobuf message on the way back in -- so every queued GroupUpdated (member-left, promote, info-change, member-change, invite-response) was written, failed to deserialize on the next launch, and had its row deleted. Nothing leaked and nothing was reported: the message was simply never sent. A leave interrupted by a restart therefore did nothing at all. Protobuf fields now round-trip through their own wire format, and both message jobs share one configured Kryo so the write and read directions cannot drift apart again.
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
1 participant
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.
MessageSendJobpersisted its message with a bareKryo. Kryo builds objects field by field, and parts of a protobuf message's object graph have no constructor it can call — so it writes one happily and then throws on the way back in:Factory.createcatches that, returns null, andJobQueue.resumePendingJobsdeletes the row. Nothing leaks and nothing is reported — but noGroupUpdatedqueued for send survives an app restart: member-left, promote, info-change, member-change and invite-response are all silently dropped. One reported log carried 552 of these.Changes
JobKryo.kt:jobKryo()registers aSerializer<MessageLite>that round-trips protobuf throughtoByteArray()/parseFrom(), which is the only representation protobuf guarantees it can rebuild.MessageSendJobandAttachmentUploadJobboth use it, in both directions, so the write and read sides cannot drift apart.No compatibility concern: a row containing a non-null protobuf field could not be read before this change either, so there is nothing already on disk that this stops being able to read.
Tests
JobKryoTestround-trips aGroupUpdatedthrough the same Kryo the job uses. It fails without the serializer registration (with the exception above) and passes with it. Full suite green: 299/299.Notes for review
The leave-retry-loop fix (separate PR) touches a different region of
MessageSendJob; the two branches merge cleanly and can land in either order. Without this one, a leave interrupted by a restart silently does nothing.