ReviewOS

also looking at this

craft-native/craft

feat(android): the reply channel, so async actions become reachable

#152
Merged chris wants to merge feat/android-reply-channel into main
5 files +464 -0
9 days ago

39 of CraftBridge.kt's 103 methods reply through runOnUiThread { webView.evaluateJavascript(…) } and nothing else. None can move to Zig until that path does — which makes this the single piece of infrastructure between seventeen migrated actions and most of the rest.

Zig cannot make a Runnable, so Kotlin keeps the threading

evaluateJavascript is main-thread-only and the hop is Activity.runOnUiThread(Runnable). JNI can only produce a Runnable by registering natives on a class that already exists in the APK — which means shipping Kotlin in order to avoid writing Kotlin.

So the hop stays in Kotlin. CraftNative.deliver(script) does it; Zig calls it as a static method. Zig owns what to say, Kotlin owns which thread says it.

The lambda CraftBridge installs is its own runOnUiThread/evaluateJavascript pair — the same one sendEvent already uses — so both sides deliver through one implementation rather than two that have to agree about threading.

The env is per-thread; the VM is not

A JNIEnv belongs to one thread and caching it across threads is the classic JNI crash. A JavaVM is process-wide. So every call asks the VM for this thread's env and attaches when it has none — the normal case for a device callback arriving on a framework thread, exactly like a CLLocationManager delegate on iOS.

A thread this file attaches, this file detaches. Leaving one attached keeps the JVM's thread record alive after the OS thread is gone, and the crash surfaces somewhere unrelated much later.

mutationresult
attached thread never detachedthe detached-thread test fails
an already-attached thread wrongly detachedthe attached-thread test fails
the settle guard droppedthe settle test fails

Both directions matter: detaching a thread the JVM created is as wrong as failing to detach one we made.

Failure is counted, never logged and never thrown

The paths that drop are the ones a device takes in bulk — no VM before load, no WebView before the page exists. A log line each would drown the thing being diagnosed. droppedCount() is what a diagnosis reads instead, the same shape ios_events.zig uses for its queue.

Tested: no VM, a thread that can't attach, and a missing holder class — each counted once, and the un-attachable thread is never detached.

settle guards the global it calls

window._craftLocationResolve && window._craftLocationResolve({...})

Not defensive noise. These globals are assigned inside a promise executor, so between an action being called and its promise being built the global doesn't exist — and a callback arriving in that window throws a ReferenceError inside evaluateJavascript, where nothing sees it. The Kotlin writes the same guard for the same reason.

No consumer yet, deliberately

This is infrastructure; the first async action is its own change. Nothing here claims anything to a page — which is the line that matters. An unreachable channel is not the same as a surface that reports success without doing the work.

zig build test:android → 124 tests, 11/11 steps · both .so build · generator 10/10.

5 changed files on the files tab, with 0 review threads.

Sign in to comment on this pull request.