Avoid pathological QT text/markdown behavior… #886

1. 
   davidgumberg commented at 11:30 pm on September 3, 2025: contributor

   …during text selection by only setting plaintext mime data.

   Fixes the OOM described in #887.

   The issue is related to the construction of the text/markdown MIME data for the selection. Using the heaptrack utility, I observed that nearly all of the allocations when reproducing happen in QTextMarkdownWriter::writeFrame. I am not 100% sure what is causing this issue in QT’s conversion of our HTML to markdown; I have tried changing the HTML tags (e.g. using <p></p> and <ul><li></li></ul> in place of tables) used in our rpcconsole messages, but the issue recurs.

   The solution applied here is to override createMimeDataFromSelection() to avoid construction of the (likely never-used anyways) text/markdown mime data, and only set plaintext mime data in the clipboard.

2. 
   DrahtBot commented at 11:30 pm on September 3, 2025: contributor

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

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   ACK	hebasto
   Concept ACK	w0xlt
   Approach ACK	katesalazar

   If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

   LLM Linter (✨ experimental)

   Possible typos and grammar issues:

   • QT’s -> Qt’s [proper capitalization of the Qt framework name]
   • mime -> MIME [acronym should be uppercase for clarity]
   • , this avoids -> . This avoids [comma splice; split into two sentences for grammatical correctness]

   drahtbot_id_5_m

3. 
   w0xlt commented at 11:33 pm on September 3, 2025: contributor

   Concept ACK
4. 
   maflcko commented at 6:14 am on September 4, 2025: contributor

   (ci failure can be ignored, see https://github.com/bitcoin/bitcoin/issues/33293)
5. 
   katesalazar commented at 4:35 pm on September 4, 2025: contributor

   […] I am not 100% sure what is causing this issue in QT’s conversion of our HTML to markdown […]

   You implying that an upstream bug must be searched in Qt?

6. 
   katesalazar commented at 4:39 pm on September 4, 2025: contributor

   Thank you

   Approach ACK

   possible nit:

   0PlainCopyQTextEdit
   1         ^
   

7. 
   katesalazar commented at 4:46 pm on September 4, 2025: contributor

   nit: splash post in two paragraphs (none tiny) then the commit message is a one liner
8. 
   davidgumberg commented at 6:55 pm on September 4, 2025: contributor

   You implying that an upstream bug must be searched in Qt?

   “Bug” might be too strong, having one HTML element be so large is probably pretty unsually behavior for users of QTextEdit, but I can’t say for sure what is going wrong in QTextMarkdownWriter::writeFrame.

   possible nit:

   0PlainCopyQTextEdit
   1         ^
   

   I don’t really agree, I figure that it’s best not to use Q when overriding a QT base class since this makes it obvious to users of PlainCopyTextEdit that it’s custom and not from upstream.

   nit: splash post in two paragraphs (none tiny) then the commit message is a one liner

   I’m not sure I follow what you’re suggesting.

9. hebasto renamed this:
   gui: Avoid pathological QT text/markdown behavior...
   Avoid pathological QT text/markdown behavior...
   on Sep 5, 2025
10. 
    hebasto commented at 11:16 pm on September 5, 2025: member

    Concept ACK.
11. hebasto added this to the milestone 30.0 on Sep 5, 2025
12. 
    hebasto commented at 2:16 pm on September 6, 2025: member

    Tested 5cb39181019dd1d132da9e87e88921e477facbae on Ubuntu 24.04 (Qt 6.4.2).

    The patch indeed fixes the pathological memory usage. However, it modifies the clipboard content. The copy-pasted prompt appears as follow:

    • on the master branch:

    0Welcome to the Bitcoin Core RPC console.
    1Use up and down arrows to navigate history, and Ctrl+L to clear screen.
    2Use Ctrl++ and Ctrl+- to increase or decrease the font size.
    3Type help for an overview of available commands.
    4For more information on using this console, type help-console.
    

    • with this PR:

    0Welcome to the Bitcoin Core RPC console.<0x2029>
Use up and down arrows to navigate history, and Ctrl+L to clear screen.
<0x2029>Use Ctrl++ and Ctrl+- to increase or decrease the font size.
<0x2029>Type help for an overview of available commands.
<0x2029>For more information on using this console, type help-console.
    

    (<0x2029> characters have been visualised manually)

13. hebasto removed this from the milestone 30.0 on Sep 6, 2025
14. Ari4ka approved
15. gui: Avoid pathological QT text/markdown behavior...

    during text selection by only setting plaintext mime data.

    6a371b70c8
16. 
    davidgumberg commented at 6:22 pm on September 9, 2025: contributor

    The patch indeed fixes the pathological memory usage. However, it modifies the clipboard content. The copy-pasted prompt appears as follow:

    […]

    Thanks for catching this, I should have checked that clipboard contents actually were reasonable with my change, my mistake.

    I’ve updated the branch to fix both the newline issue, and the issues that existed with copying icon’s and some of the other rich text stuff in the console. I use the same function as upstream to convert the selected QTextDocumentFragment to plaintext, so clipboard contents should be identical to master, I’ve checked a few simple cases and they look right to me.

17. davidgumberg force-pushed on Sep 9, 2025
18. 
    w0xlt commented at 6:43 pm on September 9, 2025: contributor

    I wasn’t able to detect any memory increase on macOS when running getblocktemplate ‘{“rules”: [“segwit”]}’ in the GUI. Is this issue specific to Linux?
19. 
    davidgumberg commented at 6:50 pm on September 9, 2025: contributor

    I wasn’t able to detect any memory increase on macOS when running getblocktemplate ‘{“rules”: [“segwit”]}’ in the GUI. Is this issue specific to Linux?

    Are you running from a release or a depends build? This will only occur when linking against a QT with the textmarkdownwriter feature enabled, which we disable in our depends build of QT. See discussion here: #887 (comment)

20. 
    davidgumberg commented at 9:08 pm on September 9, 2025: contributor

    CI Failure is unrelated to this PR: https://github.com/bitcoin/bitcoin/issues/33345
21. hebasto approved
22. 
    hebasto commented at 11:22 am on September 10, 2025: member

    ACK 6a371b70c87ad6b763c89384562fce8549f37434.
23. hebasto merged this on Sep 10, 2025
24. hebasto closed this on Sep 10, 2025

    ---

25. 
    hebasto commented at 11:28 am on September 10, 2025: member

    @fanquake

    Backport to 30.x?

26. fanquake referenced this in commit 6b19ede1a5 on Sep 10, 2025
27. 
    fanquake commented at 11:44 am on September 10, 2025: member

    Backported to 30.x in https://github.com/bitcoin/bitcoin/pull/33356.

Contributors
davidgumberg DrahtBot w0xlt maflcko katesalazar hebasto Ari4ka fanquake