Opt-in batch serialisation for shared-main-thread runners; make jsaddle-wkwebview TH-loadable; aeson <2.4 - #169
Merged
Conversation
runJavaScript gains an optional serialiser (Maybe (MVar ())) that brackets each batch round-trip (sendBatch..takeResult). runJavaScript is now `runJavaScriptWithSerializer Nothing`, so warp/terminal/CLib/null keep running contexts fully in parallel: each browser client is its own context on its own transport, and serialising across them would let one slow client stall the rest. jsaddle-wkwebview passes `Just` a module-global lock shared across all windows. They all dispatch onto the one Cocoa main queue, and a synchronous window.prompt round-trip blocks that queue, so two windows driving JS concurrently can wedge each other. The lock is safe against jsaddle's sync-callback protocol: a prompt handler returns the pre-set lastAsyncBatch without needing the batch thread, so blocking the batch thread never stalls an in-flight synchronous round-trip.
…ollision Cabal 3.12+ compiles foreign sources into the same object tree as Haskell modules, `ar` stores archive members by basename, and GHC 9.14's in-process TH loader resolves members by name — so the cbits object and the Language.Javascript.JSaddle.WKWebView module object both becoming `WKWebView.o` makes any TH splice that loads this package fail with a spurious "duplicate definition for symbol _openApp" (seen building leksah's gi-gtk TH splices).
…ader GHC's runtime linker (used for Template Haskell when GHC itself is statically linked) treats the Objective-C protocol/class metadata every ObjC object emits (e.g. __OBJC_LABEL_PROTOCOL_$_NSObject) as strong duplicate definitions — the system linker coalesces them — so loading a library with more than one ObjC archive member fails. Combine the Cocoa sources (WKWebView-cbits.m + AppDelegate.m) into one translation unit via #include when include-app-delegate is set, and move the base cxx-source into each branch so it isn't compiled twice.
AppDelegate.m's _Nonnull annotations put the combined translation unit into clang's nullability-audit mode, which then demands annotations on every pointer in WKWebView-cbits.m; both files compile cleanly standalone.
AppDelegate.m left every cxx-sources list when the composite TU was introduced, so cabal stopped including it in the sdist and the composite's #include failed with 'file not found'.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces an opt-in mechanism to serialize JSaddle batch round-trips for runners that share a single UI/main-thread transport (notably WKWebView), widens aeson upper bounds to allow 2.3.x, and adjusts jsaddle-wkwebview’s Objective-C build layout to avoid Template Haskell runtime linker failures under statically linked GHC.
Changes:
- Add
runJavaScriptWithSerializerand routerunJavaScriptthrough it to optionally serialize eachsendBatch..takeResultround-trip. - Widen
aesonupper bounds from<2.3to<2.4in several packages. - Rework
jsaddle-wkwebviewObjC sources: rename cbits file to avoid archive-member basename collisions, and compile Cocoa sources as a single translation unit; ensure#included sources are shipped viaextra-source-files.
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| jsaddle/src/Language/Javascript/JSaddle/Run.hs | Adds runJavaScriptWithSerializer and brackets the batch round-trip with an optional MVar serializer. |
| jsaddle/jsaddle.cabal | Widens aeson upper bound to <2.4. |
| jsaddle-wkwebview/src/Language/Javascript/JSaddle/WKWebView/Internal.hs | Uses a module-global lock and calls runJavaScriptWithSerializer (Just lock) to serialize batch round-trips across windows. |
| jsaddle-wkwebview/jsaddle-wkwebview.cabal | Switches Cocoa build to a single ObjC TU and adds extra-source-files for #included sources; widens aeson bound. |
| jsaddle-wkwebview/cbits/WKWebView-cbits.m | Introduces renamed WKWebView cbits implementation to avoid object-name collisions. |
| jsaddle-wkwebview/cbits-cocoa/WKWebView-AppDelegate.m | Adds composite Cocoa translation unit including the cbits and AppDelegate sources. |
| jsaddle-webview2/jsaddle-webview2.cabal | Widens aeson upper bound to <2.4. |
| jsaddle-webkitgtk/jsaddle-webkitgtk.cabal | Widens aeson upper bound to <2.4. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Three independent groups of changes, one commit each (or a small run of commits) so they can be reviewed separately.
1.
runJavaScriptWithSerializer— opt-in batch serialisation (5df2029)runJavaScriptgains an optional serialiser (Maybe (MVar ())) that brackets the batch round-trip (sendBatch..takeResult);runJavaScript = runJavaScriptWithSerializer Nothing, so nothing changes for existing runners. jsaddle-warp, -terminal, -clib and the null runner keep running their contexts fully in parallel — each browser client is its own context on its own transport, and serialising across them would let one slow client stall the rest.jsaddle-wkwebviewpassesJusta module-global lock shared by all windows, because the contended resource is shared: every window dispatches its JS onto the one Cocoa main queue, and a synchronouswindow.promptround-trip blocks that queue, so two windows driving JS concurrently can wedge each other.The lock is safe against jsaddle's sync-callback protocol: a
prompthandler returns the pre-setlastAsyncBatchwithout needing the batch thread, so blocking the batch thread never stalls an in-flight synchronous round-trip. It is also the onlysendBatchcall site in the function, so there is no unbracketed path.2.
aeson < 2.4(c8f04af,ead142a)Allows aeson 2.3.x for
jsaddle,jsaddle-wkwebview,jsaddle-webkitgtkandjsaddle-webview2.jsaddle-warp,jsaddle-clibandjsaddle-webkit2gtkstill cap at<2.3, so a project combining those with aeson 2.3 still fails to solve. I left them alone rather than widen a bound I haven't built against — say the word and I'll bump and test them in this PR.3. One Cocoa ObjC translation unit, so dependents' TH splices can load the library (
c4d992b,cdf4bee,e0d6d03,c583144)Two separate ways this package broke Template Haskell in dependents under a statically linked GHC:
arstores members by basename, and GHC 9.14's in-process TH loader resolves members by name — socbits/WKWebView.mand theLanguage.Javascript.JSaddle.WKWebViewmodule both producingWKWebView.omade any TH splice that loads this package fail with a spuriousduplicate definition for symbol _openApp(hit building leksah's gi-gtk splices). Fixed by renaming the cbits file toWKWebView-cbits.m.__OBJC_LABEL_PROTOCOL_$_NSObject) as strong duplicate definitions — the system linker coalesces them — so a library with more than one ObjC archive member won't load. The Cocoa sources are now combined into a single translation unit,cbits-cocoa/WKWebView-AppDelegate.m, which#includes both; the base cbits source moved into each conditional branch so it is never compiled twice. Combining them puts the whole TU into clang's nullability-audit mode (AppDelegate.m uses_Nonnull), which then demands annotations on every pointer in the other file, so the composite silences the two completeness diagnostics — both files still compile cleanly standalone.#included rather than compiled directly, they left everycxx-sourceslist and cabal stopped shipping them, so the composite's#includefailed with file not found from a tarball. They are listed inextra-source-filesnow.The iOS/UIKit branch and the
include-app-delegate: Falsebranch keep compilingcbits/WKWebView-cbits.mdirectly.Testing
jsaddleandjsaddle-wkwebviewbuild clean on macOS/aarch64, GHC 9.14.cabal sdist jsaddle-wkwebviewnow containscbits/WKWebView-cbits.m,cbits-cocoa/AppDelegate.mandcbits-cocoa/WKWebView-AppDelegate.m— i.e. the composite#includeresolves from a released tarball.jsaddle) and group 2, but never compiles the wkwebview ObjC.