Skip to content

Bug 2050036 - Add Store::shutdown() to close the autofill DB connection#7490

Open
DimiDL wants to merge 1 commit into
mozilla:mainfrom
DimiDL:passport-autofill-shutdown
Open

Bug 2050036 - Add Store::shutdown() to close the autofill DB connection#7490
DimiDL wants to merge 1 commit into
mozilla:mainfrom
DimiDL:passport-autofill-shutdown

Conversation

@DimiDL

@DimiDL DimiDL commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

The connection is otherwise only closed when the Store is dropped, which on Firefox Desktop happens during GC at shutdown -- past the late-write barrier, so the flush crashes debug builds.

Wrap the connection in Mutex<Option> behind a fallible lock_db() helper (mirroring LoginStore); shutdown() closes it early, and later operations return DatabaseClosed.

Pull Request checklist

  • Breaking changes: This PR follows our breaking change policy
    • This PR follows the breaking change policy:
      • This PR has no breaking API changes, or
      • There are corresponding PRs for our consumer applications that resolve the breaking changes and have been approved
  • Quality: This PR builds and tests run cleanly
    • Note:
      • For changes that need extra cross-platform testing, consider adding [ci full] to the PR title.
      • If this pull request includes a breaking change, consider cutting a new release after merging.
  • Tests: This PR includes thorough tests or an explanation of why it does not
  • Changelog: This PR includes a changelog entry in CHANGELOG.md or an explanation of why it does not need one
    • Any breaking changes to Swift or Kotlin binding APIs are noted explicitly
  • Dependencies: This PR follows our dependency management guidelines
    • Any new dependencies are accompanied by a summary of the due diligence applied in selecting them.

mhammond
mhammond previously approved these changes Jul 22, 2026
Comment thread components/autofill/src/db/store.rs Outdated
// This is the type that uniffi exposes.
pub struct Store {
pub(crate) db: Mutex<AutofillDb>,
pub(crate) db: parking_lot::Mutex<Option<AutofillDb>>,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

fwiw, many of our crates use parking_lot::Mutex and typically just use it directly rather than fully qualifying it in each use. I'm not against doing it this way, but fyi

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

thank you for the review! Updated to follow the convention.

The connection is otherwise only closed when the Store is dropped, which on
Firefox Desktop happens during GC at shutdown -- past the late-write barrier,
so the flush crashes debug builds.

Wrap the connection in Mutex<Option<AutofillDb>> behind a fallible lock_db()
helper (mirroring LoginStore); shutdown() closes it early, and later operations
return DatabaseClosed.
@DimiDL
DimiDL force-pushed the passport-autofill-shutdown branch from 3fedaa3 to 12f51d6 Compare July 22, 2026 05:56
@mergify
mergify Bot dismissed mhammond’s stale review July 22, 2026 05:56

The pull request has been modified, dismissing previous reviews.

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.

2 participants