Qt: pass clientmodel changes from walletframe to walletviews #18091

1. 
   jonasschnelli commented at 8:43 AM on February 7, 2020: contributor

   Fixes #18090

   We currently don't pass clientmodel changes from the walletframe to the walletviews leading to possible invalid access during shutdown because all walletviews miss the nullifying of the clientmodel.

   TODO: needs investigation if this is should be backported.

2. jonasschnelli added the label GUI on Feb 7, 2020
3. jonasschnelli requested review from fanquake on Feb 7, 2020
4. 
   MarcoFalke commented at 2:21 PM on February 7, 2020: member

   This only affects tests (generateto*), so doesn't need backport?

5. 
   jonasschnelli commented at 3:03 PM on February 7, 2020: contributor

   Very likely only generateto* calls that are affected. Though there could be other calls that have this shutdown race.

6. in src/qt/walletframe.cpp:42 in ce2e941f6b outdated

     36 | @@ -37,6 +37,10 @@ WalletFrame::~WalletFrame()
     37 |  void WalletFrame::setClientModel(ClientModel *_clientModel)
     38 |  {
     39 |      this->clientModel = _clientModel;
     40 | +
     41 | +    QMap<WalletModel*, WalletView*>::const_iterator i;
     42 | +    for (i = mapWalletViews.constBegin(); i != mapWalletViews.constEnd(); ++i)
   

   ---

   ---

   

   promag commented at 3:28 PM on February 7, 2020:

   nit, auto i =, also add {.

   ---

   

   jonasschnelli commented at 5:41 PM on February 7, 2020:

   fixed.

7. 
   promag commented at 3:36 PM on February 7, 2020: member

   Code review ACK ce2e941f6b4216d40fcafd5ad942082318747469.

   Follow up, also because lately I've been addressing removing circular dependencies, I think we should make clear the lifecycle of these objects. For instance, considering the following order of instantiation ClientModel -> BitcoinGUI -> WalletFrame -> WalletModel -> WalletView then we could drop these setClient and setModel and receive the necessary objects in the constructors. Then destruction order should be reverse.

8. 
   emilengler commented at 5:34 PM on February 7, 2020: contributor

   tACK ce2e941f6b4216d40fcafd5ad942082318747469

   Only affected me when contacting through the RPC HTTP Server, not through the Qt console.

9. Qt: pass clientmodel changes from walletframe to walletviews 2af3e16ca9
10. jonasschnelli force-pushed on Feb 7, 2020
11. 
    laanwj commented at 10:48 AM on February 10, 2020: member

    Good catch, code review ACK 2af3e16ca917acd85c2d4f709f6d486519d6af0d

12. laanwj referenced this in commit 407d7c831a on Feb 10, 2020
13. laanwj merged this on Feb 10, 2020
14. laanwj closed this on Feb 10, 2020

    ---

15. Fabcien referenced this in commit bf384a3c47 on Dec 22, 2020
16. PastaPastaPasta referenced this in commit 5049c4bdb7 on Jun 27, 2021
17. PastaPastaPasta referenced this in commit 6650b11c9a on Jun 28, 2021
18. PastaPastaPasta referenced this in commit a6dc870292 on Jun 29, 2021
19. PastaPastaPasta referenced this in commit 9399a98956 on Jul 1, 2021
20. PastaPastaPasta referenced this in commit 2c05c74c18 on Jul 1, 2021
21. PastaPastaPasta referenced this in commit 4357992b16 on Jul 14, 2021
22. PastaPastaPasta referenced this in commit 7ace8e55e3 on Jul 14, 2021
23. PastaPastaPasta referenced this in commit 9cdcdf2c5d on Jul 15, 2021
24. DrahtBot locked this on Feb 15, 2022

Contributors

jonasschnelliMarcoFalkepromagemilenglerlaanwj

Review Requested
fanquake