qt: Do not block GUI thread in RPCConsole #17659

On master (35eda631ed3bd23d4a41761a85a96f925d4a6337) the GUI thread is blocked with QThread::wait() during bitcoin-qt shutdown routine. This causes unresponsive GUI if some commands are passed to the RPC console (#13217) before shutdown initiating.

This PR:

• removes blocking call and uses additional signal-to-slot connections.
• makes bitcoin-qt shutdown routine more streamlined: the only QApplication::exec() is used in bitcoin-qt. Therefore, the main event loop never interrupts until shutdown, unlike master and alternative #13674.

Refs:

• #10504
• #17145

This PR is an alternative to #13674.

Fix #13217

Fix #13217; this is an alternative to #13674 Fix #17495

As a side effect, this PR makes bitcoin-qt shutdown routine more streamlined.

Refs:

• #10504
• #17145

Could you update this with a description of what this PR does? Useful information to include:

• details about how behavior changed, what behavior was before and after this change
• a short description of the code changes, what was wrong with the old code, and how the commits relate to each other
• how this compares to the alternative pr

It's great to link to other issues and pull requests in a PR description for extra details, but it shouldn't be necessary to open multiple other issues just to have a basic understanding of what a PR does and what motivated it

<!--e57a25ab6845829454e8d69fc972939a-->

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

<!--174a7506f384e20aa4161008e828411d-->

Conflicts

No conflicts as of last run.

Calling waitfornewblock in the GUI console returns immediately.

Calling waitfornewblock in the GUI console returns immediately.

I have observed such behavior on regtest only once, and cannot reproduce anymore.

Could you provide steps to reproduce an issue reliably?

@ryanofsky

OP has been updated.

@hebasto actually it's unrelated to this change. Just noticed that waitfornewblock returns immediately in the GUI console because IsRPCRunning is false unless you start with -server.

The folllowing still hangs:

• bitcoin-qt -regtest -server
• open gui console
• run waitfornewblock
• try to quit

Are you able to reproduce?

@promag

The folllowing still hangs:

* `bitcoin-qt -regtest -server`

* open gui console

* run `waitfornewblock`

* try to quit

This requires a forced interruption of the RPC thread. It seems a bit out of the scope of this PR, as it solves the blocking of the GUI thread only.

In your example bitcoin-qt literally "waits for new block", which is unreachable in regtest after quit is initiated ;) But this PR works on mainnet and testnet: bitcoin-qt quits just after a new block arrives.

Maybe, to mention in OP that #17495 is fixed, is wrong, no?

@hebasto maybe just mention it's not a complete fix.

@promag

@hebasto maybe just mention it's not a complete fix.

The mention about the fix of #17495 has been removed from PR description.

The actual fix of #17495 is #18452 ;)

Rebased 485e3dea08d291f1b32ba94e09ee793440f4daec -> e7ee445b9191d37505c8711115c5ef85c86ba31f (pr17659.01 -> pr17659.02) due to the conflict with #16224.

Nice. Concept ACK

Concept ACK. Needs rebase.

Rebased e7ee445b9191d37505c8711115c5ef85c86ba31f -> 776d15763ea1551d7a5a86be29cfcd125d173d9d (pr17659.02 -> pr17659.03) due to the conflict with #16432.

Rebased 776d15763ea1551d7a5a86be29cfcd125d173d9d -> 47912c505eebc79c849cf14fcf9d2654caa0945a (pr17659.03 -> pr17659.04) due to the conflict with #19011.

Lets move this over to the GUI repo. I've also ported the original issue there now as well, given that the non-gui part has been fixed.

Moved to https://github.com/bitcoin-core/gui/pull/59.