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

JSON-RPC provide_backup call cannot be used reliably #5431

Closed
link2xt opened this issue Apr 5, 2024 · 3 comments · Fixed by #5439
Closed

JSON-RPC provide_backup call cannot be used reliably #5431

link2xt opened this issue Apr 5, 2024 · 3 comments · Fixed by #5439
Assignees
Labels
bug Something is not working jsonrpc

Comments

@link2xt
Copy link
Collaborator

link2xt commented Apr 5, 2024

There is a race condition in typical usage
of provide_backup and get_backup_qr/get_backup_qr_svg JSON-RPC APIs.

Desktop usage is here:
https://github.com/deltachat/deltachat-desktop/blob/828f472a433a56bd77c25cb6f2aebcc556828b36/src/renderer/components/dialogs/SetupMultiDevice/SendBackupDialog.tsx#L87-L89

provide_backup only returns once backup is completely transferred,
so it is called asynchronously.
Then get_backup_qr or get_backup_qr_svg is called,
but these functions return "No backup being provided" error
if provide_backup has not created ProviderQr yet.

C API does not have such problem,
there backup provider is created with
dc_backup_provider_new and then used
by dc_backup_provider_get_qr
and dc_backup_provider_get_qr_svg.
dc_backup_provider_get_qr
and dc_backup_provider_get_qr_svg
cannot fail because BackupProvider
always exist when they are called.

Minimal change that does not require API changes would be
to make get_backup_qr and get_backup_qr_svg block
until there is a provider available.

@Simon-Laux
Copy link
Contributor

I think going for the minimal api change makes sense, those calls already wait until the preparation is done anyway.

On the other hand it could be confusing for client/bot devs that don't read documentation if it blocks instead of returning an error when you haven't called provide_backup yet.

@link2xt link2xt self-assigned this Apr 6, 2024
@link2xt
Copy link
Collaborator Author

link2xt commented Apr 6, 2024

I will try to fix it by blocking in get_backup_qr.

@link2xt
Copy link
Collaborator Author

link2xt commented Apr 7, 2024

I made a PR that makes get_backup_qr always block until there is a QR code even if provide_backup has not started yet: #5439
But there is a problem with this approach that we deadlock if provide_backup fails:
#5439 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something is not working jsonrpc
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants