Getting ready to Qt 6 (6/n). Replace QCoreApplication::quit() with QCoreApplication::exit(0) #586

1. 
   hebasto commented at 2:00 pm on April 16, 2022: member

   Qt 5:

   • no behavior change.

   See https://code.qt.io/cgit/qt/qtbase.git/tree/src/corelib/kernel/qcoreapplication.cpp?h=5.15#n2012:

   0void QCoreApplication::quit()
   1{
   2    exit(0);
   3}
   

   Qt 6:

   • this change avoids sending a duplicated QEvent::Quit

   We use QEvent::Quit to handle macOS dock menu events. Qt 6 uses QEvent::Quit more widely. We do not want a duplicated QEvent::Quit which fires Assert(node.args); in the Shutdown() function.

2. hebasto added the label Qt 6 on Apr 16, 2022
3. 
   hebasto commented at 2:28 pm on April 16, 2022: member

   Just for a quick reference, here is a (full?) list of ways to quit the GUI:

   1. Close the splash screen window
   2. Press q on the splash screen
   3. Close the main window (if the “Minimize on close” option is disabled)
   4. Menu File –> Exit. On macOS use an application menu
   5. Shortcut Ctrl+Q
   6. System tray icon menu Exit. On macOS use a Dock icon menu
   7. Run stop command in the Console tab
   8. Receive the stop RPC command (if the “Enable RPC server” option is checked)
   9. Press “Reset Options” button in the Options tab
   10. Ctrl+C in a shell which runs bitcoin-qt
4. w0xlt approved
5. 
   w0xlt commented at 2:43 pm on April 19, 2022: contributor

   tACK https://github.com/bitcoin-core/gui/pull/586/commits/4717ebe71d6c8dad04be51ef00021a8586e2424f on Ubuntu 21.10, Qt 5.15.2.

   Tested with all options mentioned in #586 (comment)

6. hebasto force-pushed on Apr 20, 2022
7. 
   hebasto commented at 11:54 am on April 20, 2022: member

   Updated 4717ebe71d6c8dad04be51ef00021a8586e2424f -> e0930ad770623ffad9a83d99846d6cfc20a52e15 (pr586.01 -> pr586.02, diff):

   • added the same changes into the tests
8. in src/qt/bitcoin.cpp:311 in e0930ad770 outdated

   307@@ -308,7 +308,7 @@ void BitcoinApplication::startThread()
   308 
   309     /*  communication to and from thread */
   310     connect(&m_executor.value(), &InitExecutor::initializeResult, this, &BitcoinApplication::initializeResult);
   311-    connect(&m_executor.value(), &InitExecutor::shutdownResult, this, &QCoreApplication::quit);
   312+    connect(&m_executor.value(), &InitExecutor::shutdownResult, [] { QCoreApplication::exit(0); });
   

   ---

   ---

   

   promag commented at 2:30 pm on May 21, 2022:

   The signal shutdownResult is emitted from executor’s thread and this results in calling exit() in that thread because the connection lacks a context. But exit() must be called in the main thread: https://doc-snapshots.qt.io/qt6-dev/qcoreapplication.html#exit. Maybe:

   0connect(&m_executor.value(), &InitExecutor::shutdownResult, this, [] {
   1    QCoreApplication::exit(0);
   2});
   

   ---

   

   hebasto commented at 2:46 pm on May 21, 2022:

   ~Why? QCoreApplication::exit() is a static member function.~

   You are right.

   ---

   

   promag commented at 2:53 pm on May 21, 2022:

   Yes, but according to the doc

   Note also that this function is not thread-safe. It should be called only from the main thread

   and by using https://doc.qt.io/qt-5/qobject.html#connect-5 we ensure that lambda is called on the right thread.

   ---

   

   hebasto commented at 3:01 pm on May 21, 2022:

   Thanks! Updated.
9. qt: Replace `QCoreApplication::quit()` with `QCoreApplication::exit(0)`

   Qt 5:
    - no behavior change
   
   Qt 6:
    - this change avoids sending a duplicated `QEvent::Quit`

   252f363f2f
10. hebasto force-pushed on May 21, 2022
11. 
    hebasto commented at 3:00 pm on May 21, 2022: member

    Updated e0930ad770623ffad9a83d99846d6cfc20a52e15 -> 252f363f2feff243cae47731d59dfa1b74dd4386 (pr586.02 -> pr586.03, diff):

    • addressed @promag’s comment
12. promag approved
13. 
    promag commented at 8:38 pm on May 22, 2022: contributor

    Code review ACK 252f363f2feff243cae47731d59dfa1b74dd4386.
14. hebasto merged this on May 23, 2022
15. hebasto closed this on May 23, 2022

    ---

16. hebasto deleted the branch on May 23, 2022
17. sidhujag referenced this in commit 98b8450b60 on May 28, 2022
18. bitcoin-core locked this on May 23, 2023

Contributors
hebasto w0xlt promag

Labels
Qt 6