gui: Fix async open wallet call order #15462

1. 
   promag commented at 3:49 PM on February 22, 2019: member

   Fixes #15455. Must call OpenWalletActivity::open asynchronously only after all connections are made to the OpenWalletActivity instance, otherwise signals can be missed.

2. in src/qt/bitcoingui.cpp:399 in 01e25ff6ad outdated

    395 | @@ -396,6 +396,7 @@ void BitcoinGUI::createActions()
    396 |                      connect(activity, &OpenWalletActivity::opened, this, &BitcoinGUI::setCurrentWallet);
    397 |                      connect(activity, &OpenWalletActivity::finished, activity, &QObject::deleteLater);
    398 |                      connect(activity, &OpenWalletActivity::finished, dialog, &QObject::deleteLater);
    399 | +                    QMetaObject::invokeMethod(activity, "open");
   

   ---

   ---

   

   promag commented at 3:51 PM on February 22, 2019:

   Note, removed the unnecessary explicit Qt::QueuedConnection argument. The default Qt::AutoConnection is enough since the activity thread is not the current thread.

   ---

   

   Sjors commented at 9:03 AM on February 23, 2019:

   Nit: check the return bool, and maybe write a log message if this is false, or abort.

   ---

   

   promag commented at 9:34 AM on February 23, 2019:

   Thanks, done.

3. MarcoFalke added the label GUI on Feb 22, 2019
4. MarcoFalke added this to the milestone 0.18.0 on Feb 22, 2019
5. 
   DrahtBot commented at 6:38 PM on February 22, 2019: member

   <!--e57a25ab6845829454e8d69fc972939a-->

   The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

   <!--174a7506f384e20aa4161008e828411d-->

   Conflicts

   Reviewers, this pull request conflicts with the following ones:

   • #15478 (wip: gui: Refactor open wallet activity by promag)
   • #15204 (gui: Add Open External Wallet action by promag)

   If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

6. Sjors approved
7. 
   Sjors commented at 9:10 AM on February 23, 2019: member

   tACK 01e25ff6ad

8. gui: Fix async open wallet call order a720a98301
9. promag force-pushed on Feb 23, 2019
10. 
    fanquake commented at 7:15 AM on February 26, 2019: member

    tACK a720a98.

    Checked this fixes #15455, and that 🦹🏿‍♂️ wallets load correctly.

11. in src/qt/bitcoingui.cpp:400 in a720a98301

     395 | @@ -396,6 +396,8 @@ void BitcoinGUI::createActions()
     396 |                      connect(activity, &OpenWalletActivity::opened, this, &BitcoinGUI::setCurrentWallet);
     397 |                      connect(activity, &OpenWalletActivity::finished, activity, &QObject::deleteLater);
     398 |                      connect(activity, &OpenWalletActivity::finished, dialog, &QObject::deleteLater);
     399 | +                    bool invoked = QMetaObject::invokeMethod(activity, "open");
     400 | +                    assert(invoked);
    

    ---

    ---

    

    laanwj commented at 10:32 AM on February 27, 2019:

    When can this fail?

    ---

    

    promag commented at 10:37 AM on February 27, 2019:

    If the slot is renamed it would still compile. With Qt 5.10 it is possible to invoke like QMetaObject::invokeMethod(activity, &OpenWalletActivity::open).

    ---

    

    laanwj commented at 10:53 AM on February 27, 2019:

    Thanks! (I think an assertion is in-order then, if only a compile-time bug can trigger it)

12. 
    laanwj commented at 10:36 AM on February 27, 2019: member

    utACK a720a983015c9ef8cc814c16a5b9ef6379695817

13. laanwj merged this on Feb 27, 2019
14. laanwj closed this on Feb 27, 2019

    ---

15. laanwj referenced this in commit 6f43ed4c5a on Feb 27, 2019
16. deadalnix referenced this in commit 1eab626b5c on May 21, 2020
17. PastaPastaPasta referenced this in commit d90e81919f on Oct 25, 2021
18. pravblockc referenced this in commit b47d70de1c on Nov 18, 2021
19. MarcoFalke locked this on Dec 16, 2021

Contributors

promagSjorsDrahtBotfanquakelaanwj

Milestone
0.18.0