fix(demo): dispose sqlite persister before folder rename in wallet service

[Ugarba202]

Summary

This PR resolves a Windows-specific filesystem locking bug (PathAccessException, errno 32) that occurs during SQLite database fallback migration and wallet reseeding in bdk_demo.

Context & Root Cause

On Windows, attempting to rename (Directory.rename) or delete (Directory.delete) a directory while an open file handle exists inside it throws OS Error: The process cannot access the file because it is being used by another process (errno = 32).

During wallet reseeding in WalletService._reseedWalletToPrimarySqlite and _reseedWalletToFallbackSqlite, Persister.newSqlite held an active file descriptor on bdk.sqlite when WalletStoragePaths.replaceWalletDataWithFallback (folder rename) or deleteFallbackWalletData was called.

This change ensures that persister and fallbackPersister are explicitly disposed before any folder renaming or deletion operations occur, matching the existing lifecycle pattern in _migrateWalletToSqliteIfMissing.

Related Issues

• Addresses Windows CI/execution file-locking issues discussed in Support seamless cross-platform Justfile execution across Windows #97 and observed during local verification of chore: make justfile fully compatible with Windows/PowerShell #100.

Checks Ran

• just format (Formatted 86 files, zero formatting diffs)
• just analyze (Zero static analyzer issues across core library)
• just demo-analyze (No issues found! in bdk_demo)
• just demo-test (bdk_demo unit and service verification suites)

[Johnosezele]

One thing I'd like fixed before merge: in _reseedWalletToFallbackSqlite, the reload after replaceWalletDataWithFallback still lives inside the same try as the persist step. If rename succeeds but _walletLoadRunner throws, the catch runs _walletDisposer(wallet) and fallbackPersister.dispose() again, those are already disposed, and it tries to clean up the fallback dir that's already gone.
Please split that into two phases (persist + failure cleanup, then dispose, rename, reload outside that catch, or use a phase flag) so we don't double dispose on that edge path.

[Johnosezele]

If replaceWalletDataWithFallback succeeds but _walletLoadRunner throws, this catch still runs after wallet + fallbackPersister were disposed (lines 614–615).

[MusabYK]

tNACK

Tested on Windows 11, the ci still fails with similar error during (just demo-test)

Error output:

PathAccessException: Deletion failed, path = 'C:\Users\xxx\AppData\Local\Temp\recovery_service_test_31a23d12' (OS Error: The process cannot access the file because it is being used by another process, errno = 32)
  dart:io                                                      FileSystemEntity.delete
  test\features\wallet_setup\recovery_service_test.dart 42:16  _tearDownServices
]

[Ugarba202]

One thing I'd like fixed before merge: in _reseedWalletToFallbackSqlite, the reload after replaceWalletDataWithFallback still lives inside the same try as the persist step. If rename succeeds but _walletLoadRunner throws, the catch runs _walletDisposer(wallet) and fallbackPersister.dispose() again, those are already disposed, and it tries to clean up the fallback dir that's already gone. Please split that into two phases (persist + failure cleanup, then dispose, rename, reload outside that catch, or use a phase flag) so we don't double dispose on that edge path.

@Johnosezele missed that edge case where it could double dispose on the error path after the rename. I'm splitting the persist step and the reload step into two phases outside that catch block right now so we don't hit that. Will push the fix soon.

[Ugarba202]

tNACK

Tested on Windows 11, the ci still fails with similar error during (just demo-test)

Error output:

PathAccessException: Deletion failed, path = 'C:\Users\xxx\AppData\Local\Temp\recovery_service_test_31a23d12' (OS Error: The process cannot access the file because it is being used by another process, errno = 32)
  dart:io                                                      FileSystemEntity.delete
  test\features\wallet_setup\recovery_service_test.dart 42:16  _tearDownServices
]

@MusabYK thanks for testing on Windows and sharing the trace! Looks like recovery_service_test.dart is throwing errno = 32 because calling wallet.dispose() in the tests only frees the FFI wallet struct, but leaves the Persister SQLite handle open in memory right when _tearDownServices tries to delete the temp folder.

I'm adding persister tracking to WalletService along with disposeWallet(wallet) so when the tests clean up, both the wallet and its persister get closed before the folder deletion, i will fixed it that.

[MusabYK]

@MusabYK thanks for testing on Windows and sharing the trace! Looks like recovery_service_test.dart is throwing errno = 32 because calling wallet.dispose() in the tests only frees the FFI wallet struct, but leaves the Persister SQLite handle open in memory right when _tearDownServices tries to delete the temp folder.

I'm adding persister tracking to WalletService along with disposeWallet(wallet) so when the tests clean up, both the wallet and its persister get closed before the folder deletion, i will fixed it that.

Hi @Ugarba202 i took a look and managed to fix the bug from my end, what i'll suggest is you dont need to add persister tracking or disposeWallet() to WalletService to avoid complications, the fix is simpler, you need to explicitly dispose persister in every scope where it is created and handed to a Wallet before the method returns. the bug also exist in the wallet_sync_job_test.dart so update it and add persister.dispose in the teardown as well, like this addTearDown(persister.dispose);

[Ugarba202]

@MusabYK thanks for testing on Windows and sharing the trace! Looks like recovery_service_test.dart is throwing errno = 32 because calling wallet.dispose() in the tests only frees the FFI wallet struct, but leaves the Persister SQLite handle open in memory right when _tearDownServices tries to delete the temp folder.
I'm adding persister tracking to WalletService along with disposeWallet(wallet) so when the tests clean up, both the wallet and its persister get closed before the folder deletion, i will fixed it that.

Hi @Ugarba202 i took a look and managed to fix the bug from my end, what i'll suggest is you dont need to add persister tracking or disposeWallet() to WalletService to avoid complications, the fix is simpler, you need to explicitly dispose persister in every scope where it is created and handed to a Wallet before the method returns. the bug also exist in the wallet_sync_job_test.dart so update it and add persister.dispose in the teardown as well, like this addTearDown(persister.dispose);

Hey @MusabYK, thanks for checking this out! I kept it simple and avoided adding persister tracking or disposeWallet() to WalletService so we don't overcomplicate things.

Here is what I updated on the branch:

1. Explicitly called persister.dispose() locally right after handing the persister over to Wallet(...) (loadWalletFromRecord, _reseedWalletToPrimarySqlite, _buildAndPersistWallet), before the methods return.
2. Updated _reseedWalletToFallbackSqlite into two clean phases (as @Johnosezele requested) to avoid any chance of double-disposing when cleaning up folders on error.
3. Added addTearDown(persister.dispose); in wallet_sync_job_test.dart just like you suggested.

[MusabYK]

tACK b152a5d
All tests passed on Windows 11

[Ugarba202]

tACK b152a5d All tests passed on Windows 11

Thank you for the tACK! Hopefully, we can get @Johnosezele to take a look so we can get this merged.

[Johnosezele]

ACK b152a5d

[Johnosezele]

As of now, this PR cannot be merged because your commits are not signed.

[Ugarba202]

As of now, this PR cannot be merged because your commits are not signed.

Thanks for the heads-up! I just set up my GPG key and force-pushed the signed commits. The verified badge should be showing up now, so everything is ready to go! 👍

[Johnosezele]

ACK 99aad23

[Johnosezele]

Your initial commit 2abab35 isn't signed yet.

[Ugarba202]

Your initial commit 2abab35 isn't signed yet.

i just set update my GPG key and force-pushed the signed commits. so everything is ready to go! 👍

[Johnosezele]

reACK 9f95b60