bring execution model into the native core

[sargunv-bot]

Summary

Test plan

AI assistance

• Tools: Claude Code
• Context: Claude explored the issue network (Dart binding needs a native owner-thread dispatcher #403, Dart owner-isolate checks do not match the C API's owner-thread checks #412, Dart binding cannot expose general synchronous resource callbacks #405, Dart transfer tokens cannot report the loss of every alias #408, Dart binding has no real backend rendering coverage #409, Dart rendered feature query surface is untested #410, Dart WakeSource leaks silently because it cannot carry a finalizer #411, Disable Dart tests in CI #418), the C API boundary and render session surface, and the Android/iOS/GLFW/node threading models in the vendored submodule, then drafted the plan under maintainer direction across several rounds of design questions and external review. Implementation has not started.

[josxha]

A execution model seems like a clean solution to me. Speaking for Dart, the awkward theading becomes a bit easier when using a Flutter environment.
Historically, Flutter used two separate threads but starting with Flutter 3.29, Flutter apps on Android and iOS execute the main Dart isolate on the application's main thread. The same change was introduced for desktop in Flutter 3.35.
As far as I understand it, the lack of thread affinity for Dart isolates is primarily a limitation of a pure-Dart usage. For typical Flutter applications, this seems less of a problem to me that Flutter now executes the main isolate on the platform thread.
Would it make sense to enforce a Flutter environment rather than Dart in general? That could simplify things and might even be helpful for #410.

• https://flutter.dev/blog/whats-new-in-flutter-3-29
• https://www.youtube.com/watch?v=miW7vCmQwnw
• https://docs.flutter.dev/release/release-notes/release-notes-3.35.0

[gabbopalma]

I've been building an experimental FFI backend for flutter-maplibre-gl on Android (exp/native-ffi-android, package maplibre_gl_native): the C API driven from Dart behind the plugin's existing public API, rendering into a Flutter Texture, with real styles, gestures, offline regions, and a benchmark harness on a physical device. Everything this plan says about Dart matches what I hit in the field, so a data point and a few review notes.

Where the current model pushed me:

• I started single-isolate and it did not survive contact with profiling: heavy tile-integration render_update frames stalled input on the main isolate (~77 fps with repeated multi-frame UI stalls, vs ~90 sustained once the engine moved off it). So the whole engine (runtime, maps, sessions) went to a worker isolate.
• The worker isolate brought migration with it: it stays alive through a gettid watchdog, two local rebind patches, and a temporary bypass of the Decouple render session ownership from map owner threads #399 lifecycle guard. The migration is constant under load, not occasional: once awaited I/O ran on that isolate I measured 25 rebinds across 7 distinct OS threads in one 60-second gesture session. Dart owner-isolate checks do not match the C API's owner-thread checks #412 is structural.
• After Decouple render session ownership from map owner threads #399 decoupled session ownership, I moved render_update onto a native AChoreographer thread that owns the live session, with a mutex handover for the session-affine calls Dart still needs (feature queries, feature state, resize/replace). One-build A/B on device: UI jank down 27-60% in every gesture phase, render p99 down 12-16%. Getting the frame path out of Dart is worth a lot.

The branch carries ten local patches against the pin, which may be worth a look: a few are ordinary upstream candidates independent of this plan (surface release/replace on a live session, a whole-style JSON getter, transition options, gesture-in-progress, two small bug fixes), and the rest is exactly the scaffolding this plan retires.

The plan deletes all of that machinery, which is the best praise I can offer it. A runtime executor called from the root isolate removes the worker isolate and every rebind workaround with it (no argument on rejecting live rebinding: mine works on device, but I would much rather not carry it). A session bound to its own executor, with my vsync thread submitting render_update per pulse, replaces the hand-rolled handover; that is also my answer to open question 2 in practice: pacing stays a small native vsync service on the host side, and the session executor is what it submits to.

Four notes from that experience:

1. Round 1's benchmark: please include a vsync-paced render_update submitted to an otherwise-idle session executor. That round trip is the number display-paced hosts live on.
2. Batch event polling may deserve Round 2 rather than "if the benchmark says it matters": a bound runtime pays one round trip per poll_event, and during gestures I drain tens of coalesced camera events per frame at 90 Hz.
3. An isolate cannot sit in wait_events, so today I pace the event drain off the vsync pulse. That works; if Round 2 intends a push-style wake through the callback adapter instead, I would consume it.
4. On Dart binding has no real backend rendering coverage #409/Dart rendered feature query surface is untested #410, one thing worth weighing before writing a Vulkan/EGL bootstrap in Dart: the C ABI tests already build those contexts. Exposing that as a small test-only shared library with a couple of entry points returning the handles would let the Dart tests load it through dart:ffi instead of reimplementing the bootstrap, with no risk of the two drifting apart, and it sidesteps Metal, where creating a device from Dart means going through the Objective-C runtime. Happy to be told there is a reason that does not work.

Happy to validate Round 1/2 branches on a real device (full example gallery plus bench harness) and report numbers.

@josxha good point, and it matches what I see: in every probe I ran, the root isolate held one OS thread throughout, so with 3.29+ the calling side really does get simpler for Flutter apps. My rebind machinery above only exists because my engine lives on a worker isolate, which has no such guarantee.
Where I would push back is on treating that as a substitute for the executor. A stable thread settles who may call, not what a call costs: an unbound runtime does its work on the caller's thread, so a pump that spans a style parse lands on the app's main thread. I cannot put a number on that one yet, since my old single-isolate measurements were dominated by render_update, which I have since moved off the isolate entirely; I plan to measure the pump-only exposure next and can report it here. But the shape of the requirement worries me more than its size: "Flutter, recent enough, and only from the main isolate" would rule out worker isolates, which is ordinary Dart and, in my case, the arrangement that measured best.
I would also keep the bindings pure Dart rather than Flutter-dependent. It keeps dart test and non-Flutter hosts working, and it lets the Flutter-side integration (texture bridge, vsync pacing, gestures) stay a separate layer that evolves on its own; mine is one such layer, and it needs nothing from the bindings except that they do not assume a UI framework. For #410 specifically, a test harness along the lines of note 4 looks like the answer that does not cost the pure-Dart property.
Either way, the executor is what completes the thread merge: heavy work moves off the app's main thread, camera writes become non-blocking commands, and the merged main isolate becomes exactly the right place to call from. Once that lands I plan to delete the worker isolate entirely.

[gabbopalma]

Following up on the pump measurement I promised. I instrumented the pump path to report percentiles over 3-second windows and ran a cold style load twice on a physical device (Xiaomi 2109119DG at 90 Hz, cleared cache, OpenFreeMap Liberty via URL). The worst single pump span across the whole load was 4.5 ms and 5.7 ms in the two runs, one spike each in the first window. Steady state after load: p50 ~0.13 ms, p99 under 0.5 ms, against an 11.1 ms frame budget.

So the "a pump can span a full style parse" exposure is roughly 5 ms once per style load for a real style, an order of magnitude below what I feared.
For the plan this reads as: hosting the runtime on the main isolate would cost at most one softened frame per style change, so the executor's win is mainly correctness (deleting the thread-affinity workarounds), not rescuing the UI thread.
Caveats: URL styles, one style, one device. Happy to run other scenarios if useful.

My plan from here: once Round 2 lands in the Dart bindings, I'll drop the worker isolate from exp/native-ffi-android and move the host to the main isolate on top of the executor.

[sargunv]

For typical Flutter applications, this seems less of a problem to me that Flutter now executes the main isolate on the platform thread.

@josxha Would a typical maplibre integration own the map on the main isolate? In Compose I'm keeping the map and runtime on a separate thread, and only the render session on the main UI thread.

I'm not familiar enough with dart/flutter but I imagine if:

• the split isolate model is desirable
• and non-main isolates still migrate threads

then flutter would still benefit from the executor. But the render session (likely to be owned by main isolate / ui thread) doesn't need such machinery.

also even though the other languages don't need it in the native core, if I'm implementing this in every language ecosystem's example, I think it might as well live in the core.

Would it make sense to enforce a Flutter environment rather than Dart in general? That could simplify things and might even be helpful for #410.

For the other bindings I'm trying to be host agnostic, but Dart is a bit of a special case; I'm not aware of it being used anywhere other than flutter. So if expecting flutter in the library simplifies things significantly, I'm open to it, but if it doesn't, I do slightly prefer keeping the concerns separate; mln-ffi integrates Dart<>MapLibre, flutter sdk is a consumer of that integration.

For tests though, I think we should probably write Dart tests that run rendering through flutter. Other bindings use various libraries, whatever is most convenient.

[sargunv]

The worker isolate brought migration with it: it stays alive through a gettid watchdog, two local rebind patches

rebind is an interesting workaround for the problem. If I understand it, that essentially allows you to let the isolate own the thread-affine handle by changing the owning thread whenever the isolate migrates?

The branch carries ten local patches against the pin, which may be worth a look: a few are ordinary upstream candidates independent of this plan

Those patches are great findings. I spun out #454 #455 #456 #457 #458 #459 #460 for all the candidates that were independent of this plan and have had my agents working through them today.

[gabbopalma]

rebind is an interesting workaround for the problem. If I understand it, that essentially allows you to let the isolate own the thread-affine handle by changing the owning thread whenever the isolate migrates?

@sargunv Exactly right: mln_runtime_rebind_thread re-homes the runtime, its maps and projections to the calling thread whenever the isolate migrates (a gettid watchdog rebinds proactively, a Dart-side hook heals lazily on token mismatch). The render session is deliberately excluded: it's owned by the render thread, and rebinding it with the runtime would steal it from there.
It works, but in Dart the cleaner arrangement is simply driving everything from the main isolate, which since Flutter 3.29 is pinned to the platform thread and never migrates; the executor is what makes that affordable, so my plan is to move there and delete the rebind machinery entirely.

Would a typical maplibre integration own the map on the main isolate? In Compose I'm keeping the map and runtime on a separate thread, and only the render session on the main UI thread.

And yes, your Compose split is the same shape I ended up with: runtime and maps on a worker, only the render session on the thread that draws.

Thanks for spinning out #454-#460 so quickly, happy to test the resulting PRs on device

[jfberry]

A data point from a production host on the unbound path, and one gap I think Part C should own.

Open question 3, answered against a real server

We replaced the Node renderer in rampardos (a Pokémon GO-style tile server) with an in-process Go renderer on this C API. It is the row your Integration Scenarios table already anticipates — raster tile server, one runtime and map per worker, texture session per worker — with workers pinned via runtime.LockOSThread(), each render driven by mln_runtime_pump, and output leaving through CPU readback rather than acquire_frame.

In production on an Intel VPS with Mesa llvmpipe (software GL, no GPU), warm, real traffic: 22.7 ms p50 at scale=1 (N=707) and 33.2 ms at scale=2 (N=1236), against ~20 ms p50 for the Node renderer it replaced on the same host. Largely thanks to #392's pump and render-update coalescing, which landed after you closed our #282 in its favour — that call was right, and the measurement says so: the render-to-completion primitive we proposed there measured 24 ms p50, and the pump path reaches 22.7 ms.

But we are not a reason to keep the unbound path. Your own analysis already covers us: an executor per worker costs us an extra thread and a microsecond handoff against renders measured in tens of milliseconds, and both are noise. We do no deterministic stepping and no reproducible batch rendering, so by the standard you set — the unbound path earns its place on stepping or it goes — we do not carry it. If Round 7 finds only your tests need stepping, our existence should not weigh against removing it. We would migrate.

Two things about that migration, one of which is an argument in the executor's favour.

The executor would fix a defect our model has today

We hit this in production:

request still image: MapLibre Native status -2: map already has a pending still-image request

mln_map_request_still_image sets still_image_request_pending, and it is cleared only from the mbgl completion callback, which runs only while the run loop is advancing. A host that abandons a render before completion — client disconnect, request deadline — and then stops pumping leaves that flag set permanently. Every later request on that map returns INVALID_STATE, so the worker can never render again and its map, runtime and GL context become dead weight. Ours accumulated until restart.

The executor loop advances the runtime every iteration:

run batch;
if (a runtime is bound) run_loop->runOnce();

so an abandoned still completes on its own and clears the flag. The failure becomes transient rather than terminal. That is a real point in the executor's favour, and it is why I would rather see the still-image lifecycle designed into Part C than left for each host to rediscover.

What a still-image host cannot express today is either a cancel for an outstanding request, or failing that a way to ask whether one is outstanding. We work around it by driving the runtime on a separate bounded budget until the abandoned render settles, and retiring the worker if it does not — a lot of machinery to recover from a flag we cannot read. Part C reads correctly for display-paced hosts across C1–C5; still-image rendering is the other half of what this API gets used for and currently has no contract at all.

The one duration that is not noise

Cancellation of marshalled work does not matter for renders — a 23 ms block is nothing, as you say. It matters where duration is unbounded, and the plan already names the case: a query can wait a full style parse. For us that is dataset reload, which today broadcasts SetStyleURL across every worker. Not a blocker, but if there is intended guidance for a host whose long operation is a style load rather than a gesture, it would be worth a line — that is the only place the blocking contract would change how we are shaped.

One smaller note

The plan states server concurrency equals threads because a runtime is one per OS thread, which is right. Worth noting for a later reader that maps are not similarly constrained: several maps can share one runtime and therefore one FileSource, since the manager keys on platformContext and that is the runtime pointer. That changes the resource story for a multi-style server without touching the concurrency one. We are about to take that saving ourselves.

Happy to contribute measurements from the pure-FFI server side for Round 5. You note it is scoped from API shape rather than a profile, and we have a workload that exercises the still-image path hard.

[sargunv]

I've refined the plan now based on findings from wasm, typescript, and compose integration spikes.

@gabbopalma @jfberry Please take another look from a Dart/Go perspective. I think this version results in the better long term api but it's a bigger departure from the current shape.

[jfberry]

Looks extremely possible. A few things which jump out to me:

1. We need to ensure the message pump is not tied to display frequency when headless. We need to run as quickly as possible (we'll benchmark it for you of course)
2. Wake-up is a potential issue; when quiet for a while we'd want to ensure that the render work is serviced immediately
3. It sounds as if you would re-use sessions; we currently build quite a few render pipelines based on style,scale - I assume we would still do this

[gabbopalma]

Hi everyone, right now I don't have the time or the internet connection to check out the new plan.
But I already know I can completely trust it :)
Please go ahead without waiting for my opinion

[jfberry]

This now looks good to me: looking forward to testing! :-)

[sargunv]

cool, it's a wide sweeping change so I'm gonna queue this work behind #587 and #588

[sargunv]

phase 1 and 2 done, the runtime implementation. tested, not yet reviewed.

below is an accounting of the net lines of code so far. overall, my impression is the core is more complex, the usage (examples) are simpler, and the bindings are bigger but easier to reason about.

[sargunv-bot]

Automated multi-agent review (Claude, requested by @sargunv). Focus: architecture shape, simplification, and mergeability; bugs secondary.

Shape verdict: the rearchitecture itself looks right. The execution model (any-thread submission, commands with terminal events, operations, snapshots, notification sources, driver work) demonstrably serves the motivating hosts — Go and Dart now work without thread pinning, and the examples that fully adopted the model got materially simpler. The core executor is small and knob-free; most of the C++ growth is flat per-function code and real integration tests, not layered speculation.

Before human review, the main cleanup themes are:

1. Test regressions: the Rust (~8.4k lines) and Zig (~2.5k lines) render-session integration suites were deleted and replaced with constant-assertion unit tests, while the C#/Kotlin/Go suites were migrated. These need to be ported, not dropped.
2. Three cross-binding bugs: Python holds the GIL across blocking notification-source calls (deadlock), .NET's NotificationReceiver buffers observed endpoints unboundedly for Task-only hosts, and Swift's RuntimeEventMask is missing .commandFinished.
3. Development scaffolding in the tree: render_phase3.go, TestPhase3*, PHASE3_HEADERS, a Zig test named "Phase 3", and a test-only drain hook exported from the production runtime TU.
4. Stale old-model naming/prose: Kotlin *Offline* wrappers on generic operations, .NET "owner thread" error messages, "Channels" filenames, "pump" comments, and leftover no-op queue/mutex scaffolding in several examples.
5. Scope questions worth deciding explicitly: api/execution-manifest.json as a hand-maintained second source of truth for all 316 exports, and whether trivial getters need full operation pairs.
6. swift-map undercuts the PR's thesis: it kept the old cross-thread map-owner architecture under new names instead of driving the map from the render loop like the other desktop examples.

Inline comments below anchor the representative instances of each theme.

[sargunv-bot]

Observed-endpoint buffers grow without bound for Task-only hosts (bug)

Every internal drain (scheduled from the native notification callback or from WaitForOperationAsync) retains a ReadyEndpoint copy: operation endpoints with a waiter are enqueued to observedEndpoints, waiterless ones go to observedOperations, and non-operation endpoints are enqueued unconditionally. These collections are only emptied by a public RuntimeHandle.DrainReadyEndpoints() call, but a host that consumes this binding purely through Tasks and DrainEvents() — which is the natural .NET usage, and there is no public readiness callback that would ever prompt it to poll DrainReadyEndpoints — never makes that call, so the buffers grow one entry per notification for the life of the runtime. Consider either dropping the replay buffer (return only endpoints from the current native drain), bounding it, or exposing a readiness callback so the buffer has a guaranteed consumer.

[sargunv-bot]

Fixed in 6cfa83e. Task-backed waits no longer enter a replay buffer, completed or released operations are forgotten, and non-operation readiness remains represented by the native source. I added a regression test that completes 256 task-only barriers and verifies no operation endpoints accumulate.

— Sol

[sargunv-bot]

swift-map keeps the old cross-thread map-owner architecture (shape)

Both Swift targets still run the map on a detached owner task (MapTask) fed by an AsyncStream of camera commands, coordinated through the ~150-line Channels class (NSCondition, map publication, shutdown handshake, failure crossing). Under the new execution model, runtime/map/camera calls are any-thread, and every other desktop example (c-map, zig-map, go-map, rust-map, lwjgl-map, dotnet-map) now creates the map and submits camera commands directly from the render-loop thread with no cross-thread command channel. This looks like the old RuntimeLoop architecture renamed rather than removed: the command stream, map publication handshake, and waitForShutdown/waitForMapTaskExit machinery all exist only to serve the extra thread. Unless Swift's async binding surface genuinely forces this (and if so, that constraint deserves a comment), collapsing MapTask/Channels into direct calls from the main run loop would remove ~200 lines per target and bring swift-map in line with the other examples and the spec's architecture section.

[sargunv-bot]

Fixed in 6cfa83e. Both Swift targets now own RuntimeHandle, MapHandle, camera submission, and rendering on the main actor. MapTask and Channels are gone; camera work is serialized directly from the host loop, and teardown awaits setup, camera, and frame work before closing the render target and map state.

— Sol

[sargunv-bot]

Hand-maintained parallel classification of every exported function (shape)

This 324-entry manifest restates, in a second file, a property that the API already encodes almost everywhere: _start functions are operations, _take_result/_default/_get accessors are immediate, mln_map_add_*/set_* are commands, *_snapshot_get is a published snapshot. The CTest check keeps the name list in sync, but the category values themselves are unverifiable hand annotations — a drift-prone second source of truth. Consider deriving the category from naming conventions (with a small exception list), or from a structured tag in each function's doc comment so the classification lives next to the declaration it describes. That would shrink this file to the exceptions and make the checker verify something real.

[sargunv-bot]

Driver-work variant is dead abstraction — nothing ever inspects the alternative (shape)

RenderDriverWorkKind, the ten single-member *DriverWork structs, and the RenderDriverWorkItem variant add a lot of type ceremony that nothing consumes: the only accessor is driver_work_callbacks(), which std::visit-s every alternative identically to reach .callbacks, and there is no holds_alternative/get_if on the variant anywhere in src/render. driver_work_kind() and make_driver_work() exist solely to round-trip an operation kind into a tag that is then discarded. RetargetDriverWork::backend_payload is never populated (make_driver_work always constructs it as {{}, callbacks}) or read. A plain struct { std::function<void()> execute; std::function<void()> abandon; } queued directly would delete ~100 lines (the enum, ten structs, the variant, driver_work_kind, the make_driver_work switch, and driver_work_callbacks) with no behavior change. If the kind is anticipated for future prioritization, that can be added when a consumer exists.

[sargunv-bot]

Fixed in 6cfa83e. RenderDriverWork is now the plain execute/abandon callback pair the queue actually consumes; the unused enum, ten wrappers, variant, mapping switch, and backend payload were removed.

— Sol

[sargunv-bot]

File is named after a development milestone, not a domain (mergeability)

render_phase3.go is named after a development phase rather than what it contains (surface/texture attach entry points and render-session methods). The phase split is arbitrary from a reader's perspective — RenderSessionHandle.ptr() and its methods live here while the rest of the render types live in render.go. Please fold this into render.go, or split by domain (e.g. render_attach.go / render_session.go) if the combined file is too large.

[sargunv-bot]

Fixed in 6cfa83e. The file is now render_session.go. I also removed the trivial render_test.go constant check while restoring substantive render coverage in the bindings that had lost it.

— Sol

[sargunv-bot]

zig-map polls notifications instead of installing a scheduling callback (mergeability)

zig-map never installs a notification callback; it calls drainNotifications unconditionally on every 8 ms loop iteration. The spec's startup order requires "Create the receiver-scoped notification source and install a scheduling callback" (step 4), and the shared baseline describes drains as scheduled by that callback. Every sibling example follows this (c-map pushes an SDL user event, go-map signals a channel, rust-map uses the winit proxy). Polling works here only because the loop already spins at 125 Hz, but it makes zig-map both spec-noncompliant and a misleading reference for the callback-driven drain pattern the PR introduces.

[sargunv-bot]

Fixed in 6cfa83e. zig-map installs a notification callback that coalesces wakeups into a registered SDL user event, and the host loop drains only when woken. A push-failure flag provides recovery if SDL cannot queue the wake event.

— Sol

[sargunv-bot]

Rust render-session integration coverage was deleted, not migrated (mergeability; GitHub can't render this file's diff, so this is a file-level comment — see around line 1)

The previous version of this file carried ~50 integration tests that attached real render sessions (EGL/WGL/Metal/WebGPU harnesses) and covered the render workflow end to end: attach/detach lifecycle, sustained render loops, resize/extent generations, readback, rendered and source feature queries, feature state, cross-thread attach, and leak reporting on drop. This rewrite replaces them with six unit tests that only check struct-to-native field conversion (several of which just verify constants, which the repo's testing guidance asks us to avoid). Meanwhile the C# (RenderSessionTests.cs, ~900 lines), Kotlin (RenderSessionHandleTest.kt, ~1300 lines), and Go render suites were migrated to the new driver model in this same PR. Per the binding spec, each binding's suite should stand on its own for the domains it supports — the Rust binding now ships render sessions, frame acquisition, readback, and query APIs with no test that ever attaches a session. The new-driver render tests should be ported here the way they were for the other bindings before this merges.

[sargunv-bot]

Fixed in 6cfa83e. The Rust suite again attaches real Vulkan, OpenGL, and Metal sessions where supported and covers owned-texture rendering, frame acquisition and release, resize/readback, surface and borrowed targets, feature-state queries, sustained frames, cross-thread control, and invalid readback. The full Rust suite passes.

— Sol

[sargunv-bot]

GIL held across notification-source calls that wait for in-flight callbacks (bug; GitHub can't render this file's diff, so this is a file-level comment — see around line 880)

mln_notification_source_clear_callback, mln_notification_source_set_callback (when replacing), and mln_notification_source_close all block until every in-flight callback entry returns (per notification.h). These are called here from #[pymethods] with the GIL held, but notification_callback_trampoline begins with Python::attach, which blocks until it can acquire the GIL. If a notification fires on a native thread just as Python calls clear_notification_callback() (or close(), or replaces the callback), the native thread blocks in Python::attach waiting for the GIL while the Python thread holds the GIL waiting for that entry to return — a deadlock. wait_operation already handles this correctly with py.detach; these three call sites need the same treatment (take py: Python<'_> and wrap the native call in py.detach, releasing the mutex guards' data by copying the handle first, as the code already does).

[sargunv-bot]

Fixed in 6cfa83e. close, callback replacement, and callback clearing now copy the native source handle and use Python::detach around the blocking native call, so an in-flight trampoline can acquire the GIL and return.

— Sol

[sargunv-bot]

RuntimeEventMask has no .commandFinished member (mergeability; GitHub can't render this file's diff, so this is a file-level comment — see around line 151)

RuntimeEventMask defines a static member for every maskable bit except MLN_RUNTIME_EVENT_MASK_COMMAND_FINISHED, even though RuntimeEventType.commandFinished exists and the Kotlin binding exposes RuntimeEventMask.COMMAND_FINISHED. A host that narrows the mask (e.g. setEventMask([.offlineRegionStatusChanged])) has no way to re-select command-finished events except via allMapEvents/allRuntimeEvents. That matters beyond API parity: applyCommandFinishedEvents is how this class retires replaced resource-transform/header-transform/provider callback states, so masking out command-finished silently makes pendingResourceTransforms and friends grow without bound. Please add the member (and consider documenting on setEventMask that the command-finished bit should stay selected while runtime callbacks are in use).

[sargunv-bot]

Fixed in 6cfa83e. RuntimeEventMask.commandFinished is public, included in the all-events assertion, and setEventMask documents why it must remain selected while runtime callback registrations are installed.

— Sol

[sargunv-bot]

Render integration coverage replaced by constant-assertion tests (mergeability; GitHub can't render this file's diff, so this is a file-level comment — see around line 5)

The previous render test suite (~2,500 lines: owned-texture lifecycle and readback, resize/scale behavior, ease-through-frames, feature state, rendered/source feature queries, cluster feature extensions, descriptor validation) is gone, and none of it reappears elsewhere in the Zig suite — attachStart/requestFrame/drainFrameResults are now only referenced in src/render.zig itself. What remains are five tests that construct a struct and assert the fields they just set, which the repo test invariants explicitly disallow ("avoid trivial tests, tests that verify constants") while requiring each binding's suite to stand on its own for the domains it supports. The Python binding kept its render coverage through the migration, so this looks like an incomplete port rather than an intentional descope. The render integration tests should be ported to the new session/driver model (or the descope justified in the PR), and the placeholder tests deleted. The name "Phase 3 render driver values..." is also implementation-workflow scaffolding that shouldn't land in the tree.

[sargunv-bot]

Fixed in 6cfa83e. The Zig suite again uses real backend fixtures and covers owned-texture lifecycle, rendering/acquisition/release, resize/readback, target replacement, feature-state and rendered queries, sustained frames, cross-thread control, and invalid readback. The placeholder constant tests and stale migration helpers are gone; the full suite passes with one platform skip.

— Sol

[sargunv-bot]

Test-only hook compiled and exported from the production runtime TU (mergeability; GitHub can't render this file's diff, so this is a file-level comment — see around line 3027)

mln_test_hold_runtime_event_drain is an extern "C" test scaffold that ships in the production library unconditionally. Everything it touches (handle_table<RuntimeObject>, RuntimeEventQueueState) is reachable from runtime/runtime.hpp, which src/c_api/tests/test_support.cpp already includes — the hook can move there with the other mln_test_* shims so the released library exports no test-only symbols.

[sargunv-bot]

Fixed in 6cfa83e. mln_test_hold_runtime_event_drain now lives in src/c_api/tests/test_support.cpp with the other test shims and is no longer compiled into the production runtime translation unit.

— Sol

[sargunv-bot]

A detached OS thread is spawned per barrier/close call (shape; GitHub can't render this file's diff, so this is a file-level comment — see around line 2842)

runtime_barrier_start spawns a detached std::thread per call just to wait on terminal_condition, and close_runtime_start (line 2910) and map_close_start in map.cpp follow the same pattern. For barriers on a hot path this is a full thread create/destroy per call. Since every submission already runs a terminal callback through finish_tracked_submission, barrier completion could instead be checked there (complete any registered barrier whose prior sequences are now terminal), reserving a spawned waiter for the two close paths where blocking work is unavoidable. If the per-call thread is intentional for simplicity, a comment saying so would help; today the three sites also repeat similar thread-startup failure handling.

[sargunv-bot]

Fixed in 6cfa83e for the hot path. Barriers are registered by submission sequence and completed from existing terminal callbacks, so they create no waiter threads; cancellation removes the registration immediately. Runtime and map close retain one waiter each because teardown blocks on active submissions and invokes or joins the executor, and both sites now document that constraint.

— Sol

[sargunv-bot]

Generic operation wrappers keep stale Offline names and duplicate each other (mergeability; GitHub can't render this file's diff, so this is a file-level comment — see around line 650)

These wrappers (pollOfflineOperation, waitOfflineOperation, cancelOfflineOperation, releaseOfflineOperation, discardOfflineOperation, offlineOperationTerminalStatus, offlineOperationDiagnostic) all call the now-generic mln_operation_* C functions and are used by the generic OperationHandle and by NotificationDispatcher.operationCompleted, not just offline regions. The names predate the OfflineOperationHandle -> OperationHandle rename and now mislead. There are also true duplicates: releaseOperation (line 485) and releaseOfflineOperation both wrap mln_operation_release, and checkOperationStatus overlaps offlineOperationTerminalStatus on mln_operation_get_status. Suggest renaming to pollOperation/waitOperation/... and collapsing the duplicate pairs.

[sargunv-bot]

Fixed in 6cfa83e. The JVM helpers are now generic pollOperation, waitOperation, cancelOperation, discardOperation, operationTerminalStatus, and operationDiagnostic functions. OperationHandle and NotificationDispatcher use them, and the duplicate releaseOfflineOperation wrapper was removed in favor of releaseOperation.

— Sol

[sargunv-bot]

Dead operation_id/found parameters threaded through every offline completion (simplification; GitHub can't render this file's diff, so this is a file-level comment — see around line 824)

complete_offline_operation immediately discards operation_id and found (static_cast<void> on both), yet every offline-region callback passes them, including call sites that carefully compute true/false for found. Dropping the two parameters (and the complete_offline_operation_error pass-throughs) removes noise from all ~12 offline entry points and stops implying the values matter.

[sargunv-bot]

Fixed in 6cfa83e. operation_id and found were removed from the completion helpers and from every offline callback capture and call site.

— Sol

[sargunv-bot]

Resolved the cleanup findings in 6cfa83e:

• collapsed dead render-driver scaffolding and removed per-barrier waiter threads;
• fixed .NET endpoint retention, Python GIL blocking, Swift event-mask parity, generic Kotlin operation naming, and offline completion noise;
• simplified the Swift, Zig, Go, .NET, and LWJGL examples and removed stale phase/channel/pump terminology;
• restored and ported substantive Rust and Zig real-backend render integration coverage;
• moved the runtime drain test hook out of the production translation unit.

I retained operation pairs for getters that enter mutable MapLibre state: they must serialize after earlier commands and may need executor-owned work. Immediate getters remain appropriate for binding-owned metadata and published snapshots, so converting these reads to immediate calls would weaken ordering rather than merely simplify the surface.

Verified with the native C API suite; Rust, Zig, .NET, Python, Go, Swift, and Kotlin JVM/native binding suites; Zig/.NET/LWJGL example builds; the Go EGL example check; and parsing both Swift example targets.

— Sol

[sargunv-bot]

Follow-up after the cleanup commit (automated re-review, requested by @sargunv).

Verified against 6cfa83e: the Python GIL detach (all three notification-source sites), the .NET observed-endpoint bound, Swift .commandFinished, the per-barrier detached thread (now only runtime create/close spawn one, once per lifetime, with justifying comments), the driver-work variant collapse, the dead operation_id/found parameters, the test drain hook (moved to the test TU), phase-3 naming (only PHASE3_HEADERS remains, in the manifest checker — moot if the manifest is dropped), the Kotlin *Offline* wrapper renames, the stale owner-thread/pump prose, zig-map's SDL-event wakeup, and swift-map's move onto main-actor tasks with MapTask/Channels deleted. The restored Rust (+2.8k) and Zig (+1.3k) render suites are genuine integration coverage — real EGL/WGL/Metal/Vulkan harnesses driving attach/render/readback/resize/query/lifecycle through the new frame-demand API — and the Go removals were exactly the two trivial tests.

Remaining items, in the inline comments below plus one breadth note: the restored suites cover roughly a third of the pre-PR test count. Much of the gap is tests obsoleted by the API redesign (wrong-thread rejection, removed attach-ref APIs), but some domains have no equivalent anywhere: source/cluster/feature-extension queries, box-query viewport clipping, leak-on-drop and map-close-blocked-by-live-session enforcement, and non-OpenGL borrowed-texture positive renders in Rust. Worth either restoring or consciously deciding they're out of scope.

[sargunv-bot]

Receiver dealloc leaves the native callback pointing at freed memory (bug)

The native callback is registered with Unmanaged.passUnretained(self), and only close() clears it. If a RuntimeHandle is dropped without close() (the leak-reporter path), the receiver deallocates while the still-running native runtime keeps the callback registered; the next operation completion or event-ready notification calls takeUnretainedValue() on a dangling pointer. A deinit that calls mln_notification_source_clear_callback and closes the source would turn this use-after-free into the plain handle leak the reporter already diagnoses.

[sargunv-bot]

Fixed in b622d9e. The native callback now borrows a receiver-owned context with a weak back-reference instead of borrowing the receiver itself, and receiver deinitialization clears the callback and closes the source. This keeps the callback context valid through an in-flight teardown race while preventing calls into a deinitializing receiver. Added a regression that drops the receiver and verifies its native source was retired. — Sol

[sargunv-bot]

Two leftover constant tests and a dead fixture (simplification)

The restored suite is genuine integration coverage — thanks. Three small leftovers: caller_and_core_driver_options_are_distinct and frame_demand_copies_pacing_and_coalescing_fields are still the constant-assertion kind this cleanup removed elsewhere, and create_webgpu_borrowed_texture_session is defined but never called by any test (the old suite's webgpu_borrowed_texture_session_renders_into_a_host_texture positive test wasn't restored), so it's dead code on mln_webgpu_backend builds.

[sargunv-bot]

Fixed in b622d9e. Removed both leftover constant-assertion tests and restored the WebGPU borrowed-texture positive render/readback test, so create_webgpu_borrowed_texture_session is live coverage again. The same pass also restored the applicable source-query, clipped box-query, cluster/feature-extension, map-close/live-session, and leak-on-drop coverage. — Sol

[sargunv-bot]

Resolved the follow-up review in b622d9e: Swift notification teardown no longer leaves a callback pointing at released receiver storage; the two Rust constant tests are gone; and the remaining applicable Rust integration gaps now cover source queries, viewport-clipped box queries, clustered feature extensions, live-session map-close rejection, recoverable leak-on-drop reporting, and WebGPU borrowed-texture rendering/readback. I did not restore obsolete wrong-thread or removed attach-ref cases, and left the execution-manifest classification finding for manual triage as requested.

Verified with the full host Rust suite (108 binding tests plus core/sys), the full Swift suite (96 tests), and targeted formatter/linter checks. The Emscripten WebGPU preset built successfully and the Rust browser binary compiled/launched; the full browser run later stalled in an unrelated early map test and was stopped.

— Sol

[sargunv-bot]

Follow-up coverage is in 894b812:

• Zig now verifies that a live attached render session rejects map close and that close succeeds after detach.
• Zig static and tile map modes now drive real owned-texture still-image operations to completion and validate the produced frame.
• Zig cluster coverage now exercises the full BND-107 path: a rendered cluster feature, unsigned cluster_id, children and expansion-zoom results, and limit/offset leaf shifting.

Two inventory corrections: Rust source-feature queries and viewport box clipping are already covered inside feature_state_and_rendered_queries_copy_native_results, and the current Vulkan borrowed-target replacement tests render both before and after replacement. I did not add Metal borrowed-texture coverage because this machine cannot build or verify that backend; that remains the explicit platform-only gap. The hand-maintained exported-function classification remains untouched as requested.

Sol

[sargunv-bot]

Heads up: the head branch t3code/native-executor was deleted and recreated during today's revision, and GitHub permanently froze this PR's view at 894b8127. The branch itself is current at 4acf907a with eight further commits (generation unification + snapshot expansion, NOT_FOUND remove commands, the layer-info aggregate, synchronous projections, docs, the full 8-binding sweep, and audit fixes). Until the PR is recreated or reopened, review the branch directly: main...t3code/native-executor

[sargunv-bot]

Superseded by #631 — GitHub permanently detached this PR from its branch after the head branch was deleted and recreated (pushes no longer update it, and close/reopen did not re-attach). The branch and all further work continue in #631; review history stays here.