Skip to content

test(platform-wallet): REMOVE_WALLET_MIDPOINT_HOOK is a process-global with no cross-module exclusion, causing intermittent failures #4593

Description

@Claudius-Maginificent

Summary

REMOVE_WALLET_MIDPOINT_HOOK (packages/rs-platform-wallet/src/manager/wallet_lifecycle.rs:78) is a process-global Mutex<Option<RemoveWalletMidpointHook>>, consulted by remove_wallet (:878). One test module arms it; there is no exclusion against sibling tests in the same test binary that also call remove_wallet. Under the default parallel harness those siblings can trip a hook that was armed for a different test.

The arming side does have an RAII guard that clears the hook on drop including on panic (:1793), so this is not a leak — but the guard only bounds the duration of the arming, and concurrent tests inside that window are unprotected.

Compounding it: the affected tests derive the same wallet id from the same fixed TEST_MNEMONIC, so they contend on identity as well as on the hook.

Observed failure

Seen once during unrelated work, in a cargo test -p platform-wallet -p platform-wallet-ffi run:

  • remove_versus_recreate_tests::removal_leaves_a_generation_registered_during_it_intact
  • register_wallet_duplicate_tests::a_recreated_wallet_inherits_the_pending_fences_of_the_generation_it_replaces"the rendezvous must have re-registered the wallet"

Reproduction status — honest

Not reliably reproducible. Roughly 1 failure in ~13 runs on the branch where it appeared, 0 in 13 runs of the baseline. That is one data point, not a rate, and the sample is far too small to establish a frequency or to conclude the branch mattered. What can be said: the mechanism above explains the failure without reference to any recent change, and adding any test perturbs scheduling on a race that already exists.

Provenance

Pre-existing. The hook was introduced in #4308 (438153da39) and is unchanged since — present with identical occurrence counts before and after the work that happened to surface this. Not caused by #4586.

Suggested fix

Give the hook cross-module exclusion rather than relying on tests not overlapping — e.g. a dedicated test-wide mutex that every remove_wallet-calling test acquires, or scope the hook per-manager/per-wallet-id instead of per-process. Giving the contending tests distinct mnemonics would remove the identity collision but not the hook race, so it is a mitigation rather than a fix.

Why file it

It is unowned, intermittent, and in a shared harness — the failure mode is a red CI run on an unrelated PR, with a cause nobody looking at that PR would find.

🤖 Generated with Claude Code

https://claude.ai/code/session_01QhTi3LdR3CfAvHRb34ZB7D

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions