Repository navigation
fix(🤖): post main-thread tasks without calling into Java - #4364
YevheniiKotyrlo wants to merge 3 commits into
Conversation
On Android, runOnMainThread queued the task and, when no dispatch was scheduled, called Java's PlatformContext.notifyTaskReady to post the queue's run to the main looper, after attaching the posting thread to the JVM where it was not. Both steps allocate on Java's heap. When it was exhausted, fbjni rethrew the post's OutOfMemoryError as a JniException, which ended the app from Recorder's destructor, and asserted when the attach failed on a thread such as a render-pool one; a caller that caught the throw was left with the dispatch marked as scheduled, so nothing posted to the main thread ran again. The main thread's queue is now MainThreadTasks, one for the process as the main looper is, and the looper watches its eventfd, which PlatformContext registers once: a task queued off the main thread into an empty queue wakes it with a write, which calls no Java, and the main thread reads the count before it takes the queue. The tasks run in the looper's descriptor dispatch rather than as messages. A task handed over on the main thread runs the queue on the spot, as it did when no dispatch was pending. MainThreadTasksTest checks the wakeups on Linux.
|
The reproduction behind the Tests section, with nothing injected: Java's heap filled for real, then a post to the main thread. Each scenario ran once on each build: the example's release build (x86_64) on an Android 15 emulator (API 35),
To rerun, apply the helper below to __skiaProbe("hold-main");
__skiaProbe("fill");
const recorder = Skia.Recorder();
recorder.drawCircle({ cx: 8, cy: 8, r: 4 });
recorder.dispose(); // or the native thread's post: __skiaProbe("post-join", 1)
__skiaProbe("free");
__skiaProbe("let-main-go");The helper adds
The helperdiff --git a/packages/skia/cpp/rnskia/RNSkManager.cpp b/packages/skia/cpp/rnskia/RNSkManager.cpp
index 4c525a599..b8fa885c7 100644
--- a/packages/skia/cpp/rnskia/RNSkManager.cpp
+++ b/packages/skia/cpp/rnskia/RNSkManager.cpp
@@ -1,9 +1,17 @@
#include "RNSkManager.h"
+#include <atomic>
+#include <chrono>
#include <memory>
+#include <string>
+#include <thread>
#include <utility>
+#include <vector>
+#include <android/log.h>
+#include <fbjni/fbjni.h>
#include <jsi/jsi.h>
+#include <sys/system_properties.h>
#include "RNDawnContext.h"
#include "RNSkGraphiteTarget.h"
@@ -99,6 +107,123 @@ void RNSkManager::installBindings() {
jsi::Object::createFromHostObject(
*_jsRuntime, std::make_shared<RNJsi::NativeObjectWorkletHelpers>()));
+ // For the reproduction only: "prop" reads a system property, "post"
+ // posts a task to the main thread from a thread of its own and logs whether
+ // the post threw, as a caller that catches it would.
+ auto platformContext = _platformContext;
+ _jsRuntime->global().setProperty(
+ *_jsRuntime, "__skiaProbe",
+ jsi::Function::createFromHostFunction(
+ *_jsRuntime, jsi::PropNameID::forAscii(*_jsRuntime, "__skiaProbe"),
+ 2,
+ [platformContext](jsi::Runtime &runtime, const jsi::Value &,
+ const jsi::Value *arguments,
+ size_t) -> jsi::Value {
+ auto operation = arguments[0].asString(runtime).utf8(runtime);
+ if (operation == "prop") {
+ auto name = arguments[1].asString(runtime).utf8(runtime);
+ char value[PROP_VALUE_MAX] = {0};
+ __system_property_get(name.c_str(), value);
+ return jsi::String::createFromUtf8(runtime, value);
+ }
+ // "hold-main" keeps the main thread waiting in native code, where it
+ // allocates nothing, until "let-main-go": while Java's heap is full
+ // the post under test is then the only Java allocation in flight.
+ static std::atomic<bool> mainThreadHeld{false};
+ static std::atomic<bool> mainThreadLetGo{false};
+ if (operation == "hold-main") {
+ mainThreadHeld = false;
+ mainThreadLetGo = false;
+ platformContext->runOnMainThread([]() {
+ mainThreadHeld = true;
+ while (!mainThreadLetGo) {
+ std::this_thread::sleep_for(std::chrono::milliseconds(1));
+ }
+ });
+ while (!mainThreadHeld) {
+ std::this_thread::sleep_for(std::chrono::milliseconds(1));
+ }
+ return jsi::String::createFromUtf8(runtime, "");
+ }
+ if (operation == "let-main-go") {
+ mainThreadLetGo = true;
+ return jsi::String::createFromUtf8(runtime, "");
+ }
+ // "fill" exhausts Java's heap for real, quickly: what Runtime says
+ // is left, as one byte array, then arrays halving from 1 MiB until
+ // 16 bytes fail, kept until "free" lets them go.
+ static std::vector<jobject> heapFiller;
+ if (operation == "fill") {
+ JNIEnv *env = facebook::jni::Environment::current();
+ jclass runtimeClass = env->FindClass("java/lang/Runtime");
+ jobject javaRuntime = env->CallStaticObjectMethod(
+ runtimeClass,
+ env->GetStaticMethodID(runtimeClass, "getRuntime",
+ "()Ljava/lang/Runtime;"));
+ auto figure = [env, runtimeClass, javaRuntime](const char *name) {
+ return env->CallLongMethod(
+ javaRuntime, env->GetMethodID(runtimeClass, name, "()J"));
+ };
+ jlong left = figure("maxMemory") - figure("totalMemory") +
+ figure("freeMemory");
+ env->DeleteLocalRef(javaRuntime);
+ env->DeleteLocalRef(runtimeClass);
+ double filled = 0;
+ auto keep = [env, &filled](jsize size) {
+ jbyteArray array = env->NewByteArray(size);
+ if (array == nullptr) {
+ env->ExceptionClear();
+ return false;
+ }
+ heapFiller.push_back(env->NewGlobalRef(array));
+ env->DeleteLocalRef(array);
+ filled += size;
+ return true;
+ };
+ if (left > (2 << 20)) {
+ keep(static_cast<jsize>(left - (2 << 20)));
+ }
+ for (jsize size = 1 << 20; size >= 16;) {
+ if (!keep(size)) {
+ size /= 2;
+ }
+ }
+ return jsi::String::createFromUtf8(
+ runtime, std::to_string(static_cast<long long>(filled)));
+ }
+ if (operation == "free") {
+ JNIEnv *env = facebook::jni::Environment::current();
+ for (auto reference : heapFiller) {
+ env->DeleteGlobalRef(reference);
+ }
+ heapFiller.clear();
+ return jsi::String::createFromUtf8(runtime, "");
+ }
+ auto id = std::to_string(static_cast<int>(arguments[1].asNumber()));
+ // "post-join" waits for its thread's post, so the heap can stay
+ // full until the post is made.
+ std::thread poster([platformContext, id]() {
+ try {
+ platformContext->runOnMainThread([id]() {
+ __android_log_print(ANDROID_LOG_INFO, "RNSkia",
+ "POST_PROBE task %s ran", id.c_str());
+ });
+ __android_log_print(ANDROID_LOG_INFO, "RNSkia",
+ "POST_PROBE post %s returned", id.c_str());
+ } catch (const std::exception &error) {
+ __android_log_print(ANDROID_LOG_INFO, "RNSkia",
+ "POST_PROBE post %s threw: %s", id.c_str(),
+ error.what());
+ }
+ });
+ if (operation == "post-join") {
+ poster.join();
+ } else {
+ poster.detach();
+ }
+ return jsi::String::createFromUtf8(runtime, "");
+ }));
+
// The WebGPU JS API (RNWebGPU, navigator.gpu, constants, createImageBitmap)
// is no longer installed here: react-native-webgpu is the single WebGPU API
// surface for React Native. Graphite keeps Dawn as an internal |
|
it's interesting because I had that exact same code at some point in v2 and we had to remove it. I forgot the details why however. |
|
It was That reason doesn't carry over. Besides moving the wakeup to Java, #3538 changed how the tasks run: its
|
Problem
On Android,
runOnMainThreadisJniPlatformContext::runTaskOnMainThread, which queues the task and, when no dispatch is scheduled, wakes the main thread by calling into Java:PlatformContext.notifyTaskReadypoststhis::notifyTaskReadyNativeto a main-threadHandler, and on a native thread not attached to the JVM, such as a render-pool thread,ThreadScopeattaches it first. Both allocate on Java's heap, so when it is exhausted:OutOfMemoryErrorcomes back through fbjni as ajni::JniExceptionand leavesrunOnMainThreadintoRecorder's destructor, which posts the release of its commands from whichever thread drops the last reference. An exception leaving a destructor callsstd::terminate;FBJNI_ASSERT(result == JNI_OK)aborts.Either way the app ends. A caller that catches the throw, as #4353's main-thread scheduler does, is left with
_mainThreadDispatchScheduledset, which only the main thread's run clears, so no task posted to the main thread runs again.Solution
The main thread is woken without Java:
MainThreadTasksinRNSkMainThreadTasks.h, shared by every React instance, where there was one per platform context.PlatformContextregisters the queue's eventfd with the main looper'sMessageQueueonce per process (addOnFileDescriptorEventListener), before it makes its native context.runMainThreadTasks()reads the eventfd before taking the queue, so no task is left without a wakeup.The posting thread makes no JNI call and attaches nothing, so neither failure can happen there; a post can still throw the queue's own
std::bad_alloc._mainThreadDispatchScheduled,notifyTaskReadyExternal()andPlatformContext.notifyTaskReadygo.Two things change on the main thread:
runOnMainThreadcalled there runs the whole queue right away, asRNSkView::setPresentThread's doc expects of Android; before, it did so only while no dispatch was pending.Loopermessage logging no longer shows them.Tests
MainThreadTasksTest, a host test incpp-tests, checks the queue's wakeups across threads on Linux, where the eventfd is; elsewhere it says it is skipped.No test the repo runs can exhaust Java's heap. I reproduced the crashes in the example's release build on an Android 15 emulator, with a helper that adds
__skiaProbefor the run (it and how to rerun it are in the comment below). The helper holds the main thread in native code so that it allocates nothing, fills Java's heap with byte arrays until a 16-byte one fails, then makes the post and frees the heap:On
main:std::terminate()←RNSkia::Recorder::~Recorder()←RNSkia::JsiRecorder::releaseResources(). Under the full heap, fbjni cannot build the exception's message either.mqt_v_js, which it inherits.With this change both run on the main thread after
let-main-go; for the post, from the helper's own logs:Verification
Run locally at this commit, cpplint, lint, tsc, the unit and web tests, the library build and the JVM tests pass, the C++ tests pass on Linux, and the example's Android release build (x86_64) succeeds with no compiler warning. Not run here: the e2e suite of CI's
build-test-android.No existing issue or PR covers this: I searched for
notifyTaskReady,runOnMainThread,mainThreadDispatchScheduledandJniException destructor.Related
std::bad_alloc.JniSkiaView's present-thread runner posts through Java the same way and is not changed here.