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(deltachat-jsonrpc): block in inner_get_backup_qr #5439

Merged
merged 1 commit into from
Apr 7, 2024
Merged

Conversation

link2xt
Copy link
Collaborator

@link2xt link2xt commented Apr 6, 2024

This change avoids the race between
provide_backup changing the state from NoProvider to Pending and a call to get_backup_qr or get_backup_qr_svg. With this change get_backup_qr and get_backup_qr_svg always block until QR code is available,
even if provide_backup was not called yet.

Fixes #5431

@link2xt link2xt requested review from flub and Simon-Laux April 6, 2024 20:20
@link2xt
Copy link
Collaborator Author

link2xt commented Apr 6, 2024

The code was introduced in #4198

Copy link
Collaborator

@iequidoo iequidoo left a comment

Choose a reason for hiding this comment

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

I only think this comment should be fixed:

    /// This call will fail if there is currently no concurrent call to
    /// [`CommandApi::provide_backup`].  This call may block if the QR code is not yet
    /// ready.
    async fn get_backup_qr(&self, account_id: u32) -> Result<String> {

@link2xt
Copy link
Collaborator Author

link2xt commented Apr 7, 2024

There is still a problem of what to do if provide_backup fails. It can happen if invalid account_id is passed or BackupProvider::prepare fails for a lot of reasons like failing to bind a socket or running out of disk space.

@link2xt link2xt force-pushed the link2xt/qr-block branch from 555e3fb to f142f0c Compare April 7, 2024 08:07
@link2xt link2xt marked this pull request as draft April 7, 2024 08:16
@link2xt link2xt marked this pull request as ready for review April 7, 2024 09:38
@link2xt
Copy link
Collaborator Author

link2xt commented Apr 7, 2024

I added 10 second timeout to make sure we don't deadlock if provider fails to start or was provide_backup was not called.

This change avoids the race between
`provide_backup` changing the state from NoProvider to Pending
and a call to `get_backup_qr` or `get_backup_qr_svg`.
With this change `get_backup_qr` and `get_backup_qr_svg`
always block until QR code is available,
even if `provide_backup` was not called yet.
@link2xt link2xt force-pushed the link2xt/qr-block branch from ae0306a to 0ca29bb Compare April 7, 2024 13:48
@link2xt link2xt merged commit 9ab2c6d into main Apr 7, 2024
38 checks passed
@link2xt link2xt deleted the link2xt/qr-block branch April 7, 2024 14:20
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.

JSON-RPC provide_backup call cannot be used reliably
2 participants