Paths

Table of Contentst

Differential D5080

rpc: Make unloadwallet wait for complete wallet unload
ClosedPublic
Actions

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

Tags

None

Subscribers

Details

Reviewers

jasonbcox
deadalnix

Group Reviewers

Restricted Project

Commits

rSTAGINGa4149bf01f26: rpc: Make unloadwallet wait for complete wallet unload
rABCa4149bf01f26: rpc: Make unloadwallet wait for complete wallet unload

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

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

Fabien created this revision.Jan 27 2020, 16:39

Herald added a reviewer: Restricted Project. · View Herald TranscriptJan 27 2020, 16:39

[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.

Build Bitcoin-ABC / Diffs / Diff Testing started.

Build Bitcoin-ABC / Diffs / Diff Testing passed.

Harbormaster completed remote builds in B9156: Diff 15815.Jan 27 2020, 17:00

jasonbcox accepted this revision.Jan 27 2020, 22:44

This revision is now accepted and ready to land.Jan 27 2020, 22:44

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

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

Fabien edited the summary of this revision. (Show Details)Jan 28 2020, 09:31

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

Build Bitcoin-ABC / Diffs / Diff Testing started.

Fabien edited the summary of this revision. (Show Details)Jan 28 2020, 09:36

Build Bitcoin-ABC / Diffs / Diff Testing passed.

Harbormaster completed remote builds in B9167: Diff 15831.Jan 28 2020, 09:39

deadalnix accepted this revision.Jan 30 2020, 01:49

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

Closed by commit rABCa4149bf01f26: rpc: Make unloadwallet wait for complete wallet unload (authored by João Barbosa <joao.paulo.barbosa@gmail.com>, committed by Fabien). · Explain WhyJan 30 2020, 08:49

This revision was automatically updated to reflect the committed changes.

Fabien added a commit: rABCa4149bf01f26: rpc: Make unloadwallet wait for complete wallet unload.

Fabien added a commit: rSTAGINGa4149bf01f26: rpc: Make unloadwallet wait for complete wallet unload.Apr 7 2020, 22:42

Revision Contents
Changeset List

Path

Size

doc/

release-notes.md

2 lines

src/

2 lines

wallet/

8 lines

10 lines

9 lines

40 lines

Diff 15884

doc/release-notes.md

Loading...

src/init.cpp

Loading...

src/wallet/init.cpp

Loading...

src/wallet/rpcwallet.cpp

Loading...

src/wallet/wallet.h

Loading...

src/wallet/wallet.cpp

Loading...