From c15881e0da2e01e1e7d692eb83a9de693417a48d Mon Sep 17 00:00:00 2001 From: Thorsten Sommer Date: Mon, 14 Sep 2026 17:04:08 +0200 Subject: [PATCH] Fix web search crashing on concurrent Markdown conversions --- app/MindWork AI Studio/Tools/HTMLParser.cs | 50 +++++++++++++++++++--- 1 file changed, 44 insertions(+), 6 deletions(-) diff --git a/app/MindWork AI Studio/Tools/HTMLParser.cs b/app/MindWork AI Studio/Tools/HTMLParser.cs index a5095830..9e2ba1ad 100644 --- a/app/MindWork AI Studio/Tools/HTMLParser.cs +++ b/app/MindWork AI Studio/Tools/HTMLParser.cs @@ -1,3 +1,4 @@ +using System.Collections.Concurrent; using System.Net; using System.Net.Http.Headers; using System.Net.Sockets; @@ -14,18 +15,39 @@ public sealed class HTMLParser private const int DEFAULT_MAX_RESPONSE_BYTES = 5 * 1024 * 1024; /// - /// The HTML to Markdown converter, built once from a fixed configuration. + /// The fixed configuration every HTML to Markdown conversion runs with. /// /// - /// Shared rather than built per call: the configuration never changes, and one web search - /// converts a page per result. + /// This one is shared, because it is only ever read: a configuration holds no counters and no + /// collections which get written to. The converters reading it are not shared, see the pool + /// below. /// - private static readonly Converter MARKDOWN_CONVERTER = new(new Config + private static readonly Config MARKDOWN_CONFIG = new() { UnknownTags = Config.UnknownTagsOption.Bypass, RemoveComments = true, SmartHrefHandling = true, - }); + }; + + /// + /// The converters not currently in use, kept so that the reflection in their constructor does + /// not run for every page. + /// + /// + /// One converter per conversion rather than one for all of them: a converter tracks the + /// ancestors of the node it is at in state of its own, updates that state at every single node, + /// and does so without any synchronization. A web search converts up to four pages at the same + /// time, which let those conversions tear each other's ancestor lists apart — sometimes loudly, + /// as an index outside the bounds of an array, and sometimes quietly, as a list indented by the + /// depth another page happened to be at.

+ /// Which converter gets which page does not matter, so the pool needs no key: that ancestor + /// state is entered and left in pairs around every node, which leaves it empty once a + /// conversion returns. Nothing of a page outlives its own conversion. A key would, in fact, do + /// harm — two conversions of the same page at the same time would share one converter again. + ///

+ /// The pool holds no more converters than are ever converting at once, which is a handful. + ///
+ private static readonly ConcurrentBag CONVERTER_POOL = []; /// /// Loads a web page. @@ -238,5 +260,21 @@ public sealed class HTMLParser /// /// The HTML content to parse. /// The converted Markdown content. - public static string ParseToMarkdown(string html) => MARKDOWN_CONVERTER.Convert(html); + /// + /// The converter returns to the pool only after it converted without throwing, and that is + /// deliberately not done in a finally block: a conversion which throws leaves the ancestors it + /// entered behind, because the library does not unwind them itself. Such a converter would + /// count those ancestors into every page it is handed afterwards, so it is left to the garbage + /// collector rather than passed on. + /// + public static string ParseToMarkdown(string html) + { + if (!CONVERTER_POOL.TryTake(out var converter)) + converter = new Converter(MARKDOWN_CONFIG); + + var markdown = converter.Convert(html); + + CONVERTER_POOL.Add(converter); + return markdown; + } } \ No newline at end of file