GUI event loop should be block free

laanwj commented at 5:11 am on October 15, 2019: member

When I created the bitcoin-qt GUI I made a big mistake in its design. I copied this more or less exactly from the wxwindows GUI. I was aware of this back in the day, but was planning to fix it later. I never got around to it. Honestly speaking I don’t think I ever will.

In any case, the event loop in the main thread of a Qt program (or any GUI program, for that matter) is never supposed to block. Any operation that can take non-trivial time (even in the order of 40ms) should be executed asynchronously.

We have many such cases; not only when the user does an explicit operation such as send, but also in response to internal notiications, and automatic periodic polls for new transactions, the current balance, and so on (these can take longer due to cs_main lock contention: worst during initial sync and when catching up). Also at the start of the application. Pretty much all communication with the node and wallet happens in the GUI thread itself.

This is more “redesign” than “refactor”, anyhow, and definitely not a “good first issue” it needs to happen at some point. Even more if we want snazzy animations in the android GUI.

(ref #17112 and plenty of other “GUI not responding” issues)

laanwj added the label GUI on Oct 15, 2019

laanwj added the label Refactoring on Oct 15, 2019

laanwj renamed this:
~~GUI event loop should be lock free~~
GUI event loop should be block free
on Oct 15, 2019

laanwj added this to the milestone Future on Oct 15, 2019

ryanofsky commented at 3:04 pm on October 15, 2019: contributor

There’s also more written about this in #10504. I disagree that this is “definitely” not a good first issue. For someone with experience with GUI programming it could be a great first issue, because it requires little knowledge of bitcoin and can be implemented incrementally, in small prs targeted at specific parts of the UI.

maflcko added the label Brainstorming on Oct 15, 2019

maflcko commented at 3:13 pm on October 15, 2019: member

a good first issue

I was about to ask how this could be solved, to give new contributors and reviewers an idea of the long term goal. Though it seems you mention two approaches in #10504. If they are applicable, we might want to tag this “good first issue”.

ryanofsky commented at 3:21 pm on October 15, 2019: contributor

Article I linked to in the other issue https://doc.qt.io/archives/qq/qq27-responsive-guis.html describes more approaches in more detail, too.

maflcko added the label good first issue on Oct 15, 2019

maflcko added the label Hacktoberfest on Oct 15, 2019

laanwj commented at 7:45 pm on October 15, 2019: member

I disagree that this is “definitely” not a good first issue. For someone with experience with GUI programming it could be a great first issue, because it requires little knowledge of bitcoin and can be implemented incrementally

I’m not sure about it, I think it does require quite some idea of the inner workings (which is easy to underestimate if you’re familiar with the code base). Then again, if someone can pick this up as first issue all the better of them.

promag commented at 6:50 pm on November 2, 2019: member

#17135 is an approach that avoids blocking the GUI thread.

gab81 commented at 9:03 am on April 14, 2020: none

hello @laanwj , do you know when we can expect a fix for this or new release for this hanging error?

sometimes i think it has crashed but i leave it there and it resuscitates eventually and works fine, i bet many people will kill it though and thus messing up the wallet, etc.

been checking the website but still on 0.19.0.1 . many thanks!

fanquake removed the label Hacktoberfest on Aug 10, 2020

jaybny commented at 7:01 am on October 7, 2021: none

hi, I have extensive experience with qt/c++/blockchain/bitcoin and can potentially tackle this a first issue.

i came here while working on a proof-of-concept for new bitcoin sidechain project and plan on decorating bitcoin-qt with custom functionality specific to that.

question1: given the bitcoin QT/QML project, is this issue still relevant? as Widgets and QML are really different GUI frameworks. Would it not be best to just jump straight to developing with QML and fixing the blocking issues there?

question2 is regarding the Process-Separation project, separating out QT from the core into its own process and communicated via IPC, really fundamentally changes the look feel responsiveness and overall proper design of the QT/GUI. if this process separation is something that we are committed to doing, maybe best to start there? and maybe even merge the two efforts into 1? separate the GUI into new QML in a new process?

as a side note, from my experience trying it both ways, ( my previous project started as a separate process, communicating via ipc to QT-Widgets, and a single process with QML) , the single process was the way to go. in fact we started with ipc/widgets and refactored to single process/qml.

promag commented at 8:46 am on October 7, 2021: member

Widgets and QML are really different GUI frameworks

The major difference is that there is a separate thread for rendering the scene graph. There is still a moment where the main loop is blocked so that the scene graph is updated against the QML scene.

Using QML doesn’t magically fix this issue. The only way to fix this is to stop calling functions that can block (usually because mutexes are locked down the road or some heavy IO happens) from the main thread (the one that drives Qt event loop).

Going multi-process also doesn’t fix this, if some function blocks for IPC then the GUI will block.

jaybny commented at 9:40 am on October 7, 2021: none

Using QML doesn’t magically fix this issue. The only way to fix this is to stop calling functions that can block (usually because mutexes are locked down the road or some heavy IO happens) from the main thread (the one that drives Qt event loop).

Going multi-process also doesn’t fix this, if some function blocks for IPC then the GUI will block.

ok. qml or multi process wont fix it. but going qml or multi process will require a complete rewrite of the gui code, is what i’m saying. so during this re-write the blocking stuff can be fixed. otherwise it seems to be double the effort.

promag commented at 9:53 am on October 7, 2021: member

I don’t see a reason to not improve in bitcoin-qt. Actually, it might be the best approach going forward with QML since by then the architecture is already oriented to async calls.

jaybny commented at 10:06 am on October 7, 2021: none

I don’t see a reason to not improve in bitcoin-qt. Actually, it might be the best approach going forward with QML since by then the architecture is already oriented to async calls.

did not understand what you wrote here. first fix blocking, and then do the major redesign? or just do the major redesign?

promag commented at 10:26 am on October 7, 2021: member

Sorry for not being clear. The current bitcoin-qt is not going anywhere and so we should improve it where possible.

A GUI based on QML is another discussion that is unrelated to this issue. The project already exists at https://github.com/bitcoin-core/gui-qml.

I think that improvements to bitcoin-qt regarding this issue will benefit both multi-process and QML. It will benefit QML because the base code is the same.

jaybny commented at 10:44 am on October 7, 2021: none

The current bitcoin-qt is not going anywhere and so we should improve it where possible.

ok

I think that improvements to bitcoin-qt regarding this issue will benefit both multi-process and QML. It will benefit QML because the base code is the same.

i’ll take your word for it, but I’m skeptical… but if current single process widgets is not going anywhere, it doesn’t really matter.

gen1lee commented at 10:00 am on June 14, 2022: none

It is surprising how unprofessional are people creating bitcoin core, and how do I need to believe that this app is super secure if its devs make such super junior mistakes?

You better used electron for desktop gui if you can’t do multithreading right.

hebasto commented at 10:19 am on June 14, 2022: member

It is surprising how unprofessional are people creating bitcoin core, and how do I need to believe that this app is super secure if its devs make such super junior mistakes?

Contributions to improve the GUI are always welcome.

mrsteve0924 commented at 2:31 am on February 17, 2024: none

after 4 years why not close this? obvious no one is going to fix it.

and the issue is still happening in 2024

kravens commented at 10:17 pm on September 11, 2024: none

Screen recording: BitcoinCore-screen0.webm I’m testing 28.X release candidate branch, is this behaviour due to these lock-ups in the GUI code?

jaybny commented at 3:35 am on September 12, 2024: none

have you tested this? https://github.com/bitcoin-core/gui-qml

kravens commented at 6:48 am on September 12, 2024: none

have you tested this? https://github.com/bitcoin-core/gui-qml

Not yet, but I will try it!

maflcko commented at 8:58 am on September 12, 2024: member

What is the status of the QML GUI? IIRC it was put on hold, because the Android APK in depends couldn’t be updated for some reason, but I may be misremembering.

hebasto commented at 11:27 am on September 12, 2024: member

What is the status of the QML GUI? IIRC it was put on hold, because the Android APK in depends couldn’t be updated for some reason, but I may be misremembering.

The development continues, but the code base is not synced with this repository until re-introducing ability to build for Android.

maflcko commented at 11:57 am on September 12, 2024: member

Ok, and Android support was blocked on cmake or qt6, or was it both? (Sorry for using this thread, but I couldn’t find the tracking issue).

hebasto commented at 12:12 pm on September 12, 2024: member

Ok, and Android support was blocked on cmake or qt6, or was it both? (Sorry for using this thread, but I couldn’t find the tracking issue).

Qt 6 is a requirement for Android support.

And CMake is a requirement for Qt 6.

jaybny commented at 6:19 am on September 13, 2024: none

qt has a tool that will build on android with qmake. not sure how much work this would be, but it’s doable. ffiw

On Thu, Sep 12, 2024, 4:57 AM maflcko @.***> wrote:

Ok, and Android support was blocked on cmake or qt6, or was it both? (Sorry for using this thread, but I couldn’t find the tracking issue).

— Reply to this email directly, view it on GitHub https://github.com/bitcoin/bitcoin/issues/17145#issuecomment-2346089167, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAF5Y3S4JXK5QEVPIPNEK4LZWF6SLAVCNFSM4JAXKVXKU5DIOJSWCZC7NNSXTN2JONZXKZKDN5WW2ZLOOQ5TEMZUGYYDQOJRGY3Q . You are receiving this because you commented.Message ID: @.***>

hebasto commented at 8:37 am on September 13, 2024: member

qt has a tool that will build on android with qmake. not sure how much work this would be, but it’s doable. ffiw … On Thu, Sep 12, 2024, 4:57 AM maflcko @.> wrote: Ok, and Android support was blocked on cmake or qt6, or was it both? (Sorry for using this thread, but I couldn’t find the tracking issue). — Reply to this email directly, view it on GitHub <#17145 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAF5Y3S4JXK5QEVPIPNEK4LZWF6SLAVCNFSM4JAXKVXKU5DIOJSWCZC7NNSXTN2JONZXKZKDN5WW2ZLOOQ5TEMZUGYYDQOJRGY3Q . You are receiving this because you commented.Message ID: @.>

The problem is #29360, which is required to support C++20.

jaybny commented at 3:26 pm on September 17, 2024: none

ok. is C++20 a requirement? why? it’s not fully supported by clang or gcc yet. can get 90% there with libstdc++ , but I guess it’s only a matter of time.

hebasto commented at 3:29 pm on September 17, 2024: member

is C++20 a requirement?

Yes. Please refer to #28349.

GUI event loop should be block free #17145