Page MenuHomePhabricator

rpc: Make unloadwallet wait for complete wallet unload
ClosedPublic

Authored by Fabien on Jan 27 2020, 16:39.

Details

Summary
Currently the unloadwallet RPC is asynchronous, it only signals the
intent to unload the wallet and then returns the response to the client.
The actual unload can happen later and the client has no way to be
notified of that.

This PR makes the unloadwallet RPC synchronous, meaning that it blocks
until the wallet is fully unloaded.

This is expected to fix wallet_multiwallet.py failure with TSAN.
Also include an undefined behavior issue, where the wallet pointer
is used after freed (see https://github.com/bitcoin/bitcoin/issues/16668).

Backport of PR14941 and PR16716.

Depends on D5079.

Test Plan
ninja check check-functional

Run the CI build-tsan configuration several times, check that the
wallet_multiwallet failure has gone.

Diff Detail

Repository
rABC Bitcoin ABC
Branch
PR14941
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 9167
Build 16288: Default Diff Build & Tests
Build 16287: arc lint + arc unit

Event Timeline

[Bot Message]
One or more PR numbers were detected in the summary.
Links to those Bitcoin Core PRs have been inserted into the summary for reference.

This revision is now accepted and ready to land.Jan 27 2020, 22:44
deadalnix requested changes to this revision.Jan 27 2020, 22:45
deadalnix added a subscriber: deadalnix.

This introduce several bugs, cf the discussion on the PR.

For instance https://github.com/bitcoin/bitcoin/issues/15310

You need to have an attack plan here.

This revision now requires changes to proceed.Jan 27 2020, 22:45
Fabien planned changes to this revision.Jan 28 2020, 07:33

Thanks for pointing this out, I'll see how to mitigate the issues.

Fabien edited the summary of this revision. (Show Details)

Include bugfix from PR16716.
This should be landed after D5085 and D5086 to avoid exposing bugs.

This revision is now accepted and ready to land.Jan 30 2020, 01:49