Skip to content

Conversation

hebasto
Copy link
Member

@hebasto hebasto commented Dec 3, 2019

On master (35eda63) 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:

Refs:

This PR is an alternative to #13674.

Fix #13217

@fanquake fanquake added the GUI label Dec 3, 2019
@ryanofsky
Copy link
Contributor

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

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

Refs:

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

@DrahtBot
Copy link
Contributor

DrahtBot commented Dec 4, 2019

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

Conflicts

No conflicts as of last run.

@promag
Copy link
Contributor

promag commented Dec 4, 2019

Calling waitfornewblock in the GUI console returns immediately.

@hebasto
Copy link
Member Author

hebasto commented Dec 4, 2019

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?

@hebasto
Copy link
Member Author

hebasto commented Dec 4, 2019

@ryanofsky

OP has been updated.

@promag
Copy link
Contributor

promag commented Dec 4, 2019

@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.

@promag
Copy link
Contributor

promag commented Dec 15, 2019

The folllowing still hangs:

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

Are you able to reproduce?

@hebasto
Copy link
Member Author

hebasto commented Dec 16, 2019

@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?

@promag
Copy link
Contributor

promag commented Feb 3, 2020

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

@hebasto
Copy link
Member Author

hebasto commented Mar 27, 2020

@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 ;)

@hebasto
Copy link
Member Author

hebasto commented May 9, 2020

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

@maflcko
Copy link
Member

maflcko commented May 23, 2020

Nice. Concept ACK

@jonasschnelli
Copy link
Contributor

Concept ACK. Needs rebase.

@hebasto hebasto force-pushed the 20191203-nonblocking-rpcconsole branch from e7ee445 to 776d157 Compare May 29, 2020 09:33
@hebasto
Copy link
Member Author

hebasto commented May 29, 2020

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

@hebasto hebasto force-pushed the 20191203-nonblocking-rpcconsole branch from 776d157 to 47912c5 Compare August 13, 2020 14:38
@hebasto
Copy link
Member Author

hebasto commented Aug 13, 2020

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

@fanquake
Copy link
Member

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.

@fanquake fanquake closed this Aug 14, 2020
@hebasto hebasto deleted the 20191203-nonblocking-rpcconsole branch August 14, 2020 10:40
@hebasto
Copy link
Member Author

hebasto commented Aug 14, 2020

Moved to bitcoin-core/gui#59.

@bitcoin bitcoin locked as resolved and limited conversation to collaborators Feb 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[GUI] QT shutdown is blocked if called before gettxoutsetinfo is finished
7 participants