Add harmless missing cs_wallet lock in qt CoinControlDialog #10340
Conversation
This is only barely a missing lock - to tickle it you have to delete from mapWallet while populating your coin control dialog. The only way (AFAIK) to do so is to call removeprunedfunds.
|
utACK 31b7171 |
|
It'd be nice to have a comment attached to cs_wallet saying when the lock needs to be acquired: Line 721 in d3dce0e Bringing this up because I think at this point if anybody would know how to write such a comment, it would probably be you. |
| // Quantity | ||
| nQuantity++; | ||
| { | ||
| LOCK2(cs_main, model->wallet->cs_wallet); |
There was a problem hiding this comment.
How can we avoid this really undesirable layer violation (model->wallet->cs_wallet)? This seems to go in the wrong direction with @ryanofsky approach to form clean layers for later process separation.
There was a problem hiding this comment.
How can we avoid this really undesirable layer violation (model->wallet->cs_wallet)?
#10244 stops using CWalletTx pointers in Qt altogether, so I think the layer violation would only be temporary. I think #10340 complements #10244 because it makes sense to fix correctness and safety issues first, and then organize the code around those changes.
| @@ -215,8 +215,9 @@ class WalletModel : public QObject | |||
|
|
|||
| bool getDefaultWalletRbf() const; | |||
|
|
|||
| CWallet *wallet; // Used to lock cs_wallet for getOutputs/listCoins | |||
|
If a non-wallet-model code-part needs to obtain the |
|
Closing in favor of #10244, which is a much better fix, though larger diff. |
This is only barely a missing lock - to tickle it you have to
delete from mapWallet while populating your coin control dialog.
The only way (AFAIK) to do so is to call removeprunedfunds.