Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix PushKit Crashes due to Undecryptable Call Invites #3989

Merged
merged 3 commits into from
Feb 2, 2021

Conversation

ismailgulek
Copy link
Contributor

@ismailgulek ismailgulek commented Feb 1, 2021

dispatch_group_leave(dispatchGroup);
}];

if (lastCallInvite.isEncrypted && ![session decryptEvent:lastCallInvite inTimeline:nil])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this code (and the full block) should be in the handleBackgroundSyncCacheIfRequiredWithCompletion completion block.
decryptEvent will return false if keys are not yet transferred from bg sync caches to the MXSession stores.

If we do that, we will probably not need to call [session.callManager handleCallEvent:lastCallInvite];.

I may be wrong but I do not understand the usage of a dispatch_group_t here and the fact we use it for one step but not all.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They're intentionally not in the completion block, because we should report the new call in the same run loop, which means we cannot do that async.

Handling to-device events seems sync (so i think decryptEvent should succeed), but handling timeline events is async, so i think it's still better to call [session.callManager handleCallEvent:lastCallInvite].

We also want to continue to sync after processing the cache, for data integrity. We're using dispatch_group_t to only wait for the completion.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it. Thanks. I failed to understand because I thought the completion block allowed to be fully async :/
Can you add this information in comments, please?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes sure

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comments added.

@ismailgulek ismailgulek merged commit 93afe78 into develop Feb 2, 2021
@ismailgulek ismailgulek deleted the element_3986 branch February 2, 2021 08:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PushKit crash
2 participants