Give console mode a real message loop, so command-line Bloom can use a browser (BL-16773) - #8261
Give console mode a real message loop, so command-line Bloom can use a browser (BL-16773)#8261JohnThomson wants to merge 1 commit into
Conversation
…a browser (BL-16773) Command-line Bloom could not reliably use a browser. Bulk upload was the visible casualty (BL-16767, fixed separately in #8246): every book after the first failed with "The instance of CoreWebView2 is uninitialized." The same trap sat under console `spreadsheetImport` of a spreadsheet carrying audio, and under anything else a console verb might want a browser for. Worse, when it sprang it said nothing useful -- the real error was thrown away, and what surfaced twenty seconds later was a navigation timeout somewhere else. The cause was the console wait loop. Console mode waited for its command by spinning `Application.DoEvents()`, and as the OUTERMOST message loop that uninstalls the WindowsFormsSynchronizationContext and leaves a plain one, whose Post queues to the thread pool. So from the first await that actually yielded, console work moved onto MTA thread-pool threads -- where a WebView2 cannot even be created, because CoreWebView2Environment.CreateAsync needs an STA thread and throws RPC_E_CHANGED_MODE. Program.Main's own comment said a synchronous Main was kept precisely to stop this happening; the wait loop immediately below it undid it. And because the browser constructors started initialization as "_ = InitWebView()", nothing ever observed the resulting exception. Nested inside Application.Run, DoEvents does not discard the context, which is why only console mode was ever affected. What this changes: - Console commands now run inside a real message loop, Program.RunConsoleCommandLoop, and are started from INSIDE it, because an await captures whichever context is current at the moment it suspends -- starting the command any earlier would already be too late for its first await. Their awaits therefore resume on the pumping STA main thread. This is what fixes the class of bug, console spreadsheet import included. Parsing and dispatch move to ParseAndDispatchConsoleCommand, unchanged apart from becoming a method. - A failed WebView2 initialization is recorded and reported rather than discarded: log, Sentry, and stderr in console mode, naming the creating thread and its apartment state. Every ready-wait now gives up at once with the real cause instead of spinning out its timeout and then blaming the timeout -- including OffScreenBrowser's own, which runs its readiness loop on the thread it owns and so cannot rely on the checks inside WebView2Browser. - SpreadsheetImporter.GetBrowserAsync no longer casts the Task<Browser> that Control.Invoke hands back to (Browser). Control.Invoke returns what the delegate returned, and the delegate is async, so the cast was an InvalidCastException waiting to happen. It now awaits the Task, as GetMd5Async in the same file already did. - Removed about forty lines of dead shared-WebView2-environment machinery (BeginSharedEnvironmentBatch / EndSharedEnvironmentBatch and their statics); its only consumer moved to OffScreenBrowser. Two comments that had explained themselves in terms of those statics are corrected, including one in ExternalApi that claimed process-book needs the UI thread "because it creates and pumps an off-screen WebView2" -- no longer the reason. The one new hazard, and what was done about it: because awaits now come back to the main thread, sync-over-async on that thread can deadlock, where under the old wait it could not. Every blocking wait reachable from a console verb was checked and is safe -- each waits on a library task that uses ConfigureAwait(false) internally (the AWS SDK, HttpClient), on work owned by another thread (OffScreenBrowser completes its own), or through AsyncUtil.RunSync, which pins to TaskScheduler.Default for exactly this reason. The one path that could have waited on the main thread cannot: ApiRequest marshals only to a form from Application.OpenForms, which is empty in console mode. RunConsoleCommandLoop's doc comment records the hazard, which calls are safe and why, and that blocking on one of Bloom's own async methods is the case to avoid. Program.MainContext is deliberately not published by the new loop, with a comment saying why: code keyed off it (RabProjectService, CommonApi, ToastService) behaves differently when it is set, and RabProjectService has a null branch precisely for the no-UI case. Tests: ConsoleCommandLoopTests covers the property that matters -- awaits resume on the calling STA thread -- and, most to the point, that a WebView2 created by a console command AFTER an await now becomes ready, which is the exact thing bulk upload could not do. It also records, as a characterization test, the WinForms behaviour the whole change is built around, so we find out if DoEvents ever stops discarding the context. WebView2BrowserInitFailureTests covers the reporting and the fast failure, including that the recorded error is readable from outside the class, which is what OffScreenBrowser depends on. Verified by hand as well as by tests: a three-book bulk upload, and a full createArtifacts run on a real book producing a valid .bloompub, an ePUB with all five page files, the complete bloomdigital folder and all three thumbnail sizes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
| Filename | Overview |
|---|---|
| src/BloomExe/Program.cs | Introduces the console command message loop and moves command parsing and dispatch inside that loop. |
| src/BloomExe/WebView2Browser.cs | Captures asynchronous WebView2 initialization failures and propagates them through browser readiness and navigation paths. |
| src/BloomExe/Publish/OffScreenBrowser.cs | Stops off-screen browser readiness polling immediately when WebView2 initialization has failed. |
| src/BloomExe/Spreadsheet/SpreadsheetImporter.cs | Awaits the Task returned by the UI-thread invocation when creating a spreadsheet-import browser. |
| src/BloomTests/ConsoleCommandLoopTests.cs | Adds regression coverage for STA continuation affinity, WebView2 readiness, and command exception propagation. |
| src/BloomTests/WebView2BrowserInitFailureTests.cs | Adds coverage ensuring initialization failures are recorded, reported, and surfaced without waiting for downstream timeouts. |
Reviews (2): Last reviewed commit: "Give console mode a real message loop, s..." | Re-trigger Greptile
|
Closing: #8251 has been retargeted to Version6.5 instead, so this cherry-pick duplicate isn't needed. |
|
Reopened — this is the PR to review/merge for BL-16773. It replaces #8251, which targeted the wrong branch ( This change was already reviewed and approved on #8251. The review history lives there: Greptile's pass, Devin consulted up to The code here is identical to what was reviewed — a straight cherry-pick of |
|
Converted to draft. Supersedes my earlier "this is the PR to review/merge" note above — that was written when 6.5 looked settled. It is not. Whether BL-16773 ships in 6.5 or waits for 6.6 is still undecided, so both candidates are open as drafts and exactly one will be merged:
They are the same change: this PR is a verified byte-for-byte cherry-pick of #8251's branch onto This PR is the 6.5 route. Merge it only if the decision is to ship in 6.5; in that case close #8251. Do not merge both. One upkeep note: |
1 participant
Problem. Command-line Bloom could not reliably use a browser. Bulk upload was the visible casualty (BL-16767, fixed separately in #8246): every book after the first failed with "The instance of CoreWebView2 is uninitialized." The same trap sat under console
spreadsheetImportof a spreadsheet carrying audio, and under anything else a console verb might want a browser for. Worse, when it sprang it said nothing useful — the real error was thrown away, and what surfaced twenty seconds later was a navigation timeout somewhere else.Cause. Console mode waited for its command by spinning
Application.DoEvents(). As the outermost message loop, that uninstalls theWindowsFormsSynchronizationContextand leaves a plain one, whosePostqueues to the thread pool — so from the firstawaitthat actually yielded, console work moved onto MTA thread-pool threads. A WebView2 cannot even be created there:CoreWebView2Environment.CreateAsyncneeds an STA thread and throwsRPC_E_CHANGED_MODE.Program.Main's own comment said a synchronousMainwas kept precisely to stop this happening; the wait loop immediately below it undid it. And because the browser constructors started initialization as_ = InitWebView(), nothing ever observed the resulting exception.Fix.
Program.RunConsoleCommandLoop), and are started from inside it, so their awaits resume on the pumping STA main thread. This is what actually fixes the class of bug, console spreadsheet import included.OffScreenBrowser's own (which runs its readiness loop on the thread it owns), now gives up at once with the real cause instead of spinning out a timeout and then blaming the timeout.SpreadsheetImporter.GetBrowserAsyncno longer casts theTask<Browser>thatControl.Invokehands back to(Browser).The one new hazard, and what was done about it. Because awaits now come back to the main thread, sync-over-async on that thread can deadlock, where under the old wait it could not. Every blocking wait reachable from a console verb was checked and is safe — each waits on a library task that uses
ConfigureAwait(false)internally, on work owned by another thread, or throughAsyncUtil.RunSync, which pins toTaskScheduler.Defaultfor exactly this reason.RunConsoleCommandLoop's doc comment records the hazard, which calls are safe and why, and that blocking on one of Bloom's own async methods is the case to avoid.Not a behavior change for the desktop app: nested inside
Application.Run,DoEventsnever discarded the context, which is why only console mode was affected.Ref: https://issues.bloomlibrary.org/youtrack/issue/BL-16773
Cherry-picked from #8251, which was mistakenly opened against
master.Devin review
This change is