Skip to content

feat: use sync interfaces for controller to avoid UniFFI Android bug#264

Open
thomas-waite wants to merge 11 commits intomainfrom
feat/use-sync-interfaces
Open

feat: use sync interfaces for controller to avoid UniFFI Android bug#264
thomas-waite wants to merge 11 commits intomainfrom
feat/use-sync-interfaces

Conversation

@thomas-waite
Copy link
Contributor

Use sync interfaces for controller to avoid UniFFI Android bug

There is a bug in UniFFI where Kotlin bindings will sometimes throw an error when calling an async Rust function: mozilla/uniffi-rs#2624

This PR updates the migration processor interfaces to use sync rather async as a work around. It has been documented that the calling mobile clients will need to take care of the call being blocking.

@thomas-waite
Copy link
Contributor Author

@codex review

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 703e819c5e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@thomas-waite
Copy link
Contributor Author

@codex review

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9606844d81

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

use once_cell::sync::Lazy;
use std::sync::Arc;
use tokio::sync::Mutex;
use std::sync::Mutex;

Choose a reason for hiding this comment

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

P2 Badge Handle poisoned migration lock explicitly

Switching MIGRATION_LOCK to std::sync::Mutex introduces lock poisoning semantics: if a migration processor panics while run_migrations() holds the guard, later try_lock() calls return Poisoned and are currently mapped to MigrationError::InvalidOperation("already in progress"). In that scenario, all subsequent migration attempts in the process will be rejected even though no run is active, which is a regression from the previous tokio::sync::Mutex behavior. Please distinguish WouldBlock from Poisoned (or recover the poisoned guard) so panics don’t permanently brick migrations.

Useful? React with 👍 / 👎.

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.

1 participant

Comments