GUI event loop should be block free #17145

issue laanwj openend this issue on October 15, 2019
  1. 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)

  2. laanwj added the label GUI on Oct 15, 2019
  3. laanwj added the label Refactoring on Oct 15, 2019
  4. laanwj renamed this:
    GUI event loop should be lock free
    GUI event loop should be block free
    on Oct 15, 2019
  5. laanwj added this to the milestone Future on Oct 15, 2019
  6. 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.
  7. maflcko added the label Brainstorming on Oct 15, 2019
  8. 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”.

  9. 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.
  10. maflcko added the label good first issue on Oct 15, 2019
  11. maflcko added the label Hacktoberfest on Oct 15, 2019
  12. 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.

  13. promag commented at 6:50 pm on November 2, 2019: member
    #17135 is an approach that avoids blocking the GUI thread.
  14. 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!

  15. fanquake removed the label Hacktoberfest on Aug 10, 2020
  16. 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.

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

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

  19. 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.
  20. 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?

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

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

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

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

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


github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-07-05 19:13 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me