Scarfski/filament
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

attempt to fix crashers when initializing the JVM

Browse Source 
- clarify precondition for VirtualMachineEnv, in particular, 
  JNI_OnLoad() must be called from a thread attached to the VM, and
  should be called once. This is now enforced internally, subsequent
  calls are ignored.

- sVirtualMachine was written and read from potentially different 
  threads without synchronization. It is now mutex protected.

- added fatal asserts in all locations that we cannot recover from;
  previously we would stay in an undefined state.

BUGS=[390724273]
This commit is contained in:
Mathias Agopian
2025-02-01 22:35:53 -08:00
committed by Mathias Agopian
parent 62cd8f1d35
commit b780cb36d0
4 changed files with 140 additions and 83 deletions
Download Patch File Download Diff File Expand all files Collapse all files
filament/backend/include/private/backend/VirtualMachineEnv.h
+14 -28
View File
@@ -18,7 +18,7 @@
#define TNT_FILAMENT_DRIVER_ANDROID_VIRTUAL_MACHINE_ENV_H
#include <utils/compiler.h>
#include <utils/debug.h>
#include <utils/Mutex.h>
#include <jni.h>
@@ -26,25 +26,17 @@ namespace filament {
class VirtualMachineEnv {
public:
// must be called before VirtualMachineEnv::get() from a thread that is attached to the JavaVM
static jint JNI_OnLoad(JavaVM* vm) noexcept;
static VirtualMachineEnv& get() noexcept {
// declaring this thread local, will ensure it's destroyed with the calling thread
static thread_local VirtualMachineEnv instance;
return instance;
}
// must be called on backend thread
static VirtualMachineEnv& get() noexcept;
static JNIEnv* getThreadEnvironment() noexcept {
JNIEnv* env;
assert_invariant(sVirtualMachine);
if (sVirtualMachine->GetEnv(reinterpret_cast<void**>(&env), JNI_VERSION_1_6) != JNI_OK) {
return nullptr; // this should not happen
}
return env;
}
// can be called from any thread that already has a JniEnv
static JNIEnv* getThreadEnvironment() noexcept;
inline JNIEnv* getEnvironment() noexcept {
assert_invariant(mVirtualMachine);
// must be called from the backend thread
JNIEnv* getEnvironment() noexcept {
JNIEnv* env = mJniEnv;
if (UTILS_UNLIKELY(!env)) {
return getEnvironmentSlow();
@@ -55,20 +47,14 @@ public:
static void handleException(JNIEnv* env) noexcept;
private:
VirtualMachineEnv() noexcept : mVirtualMachine(sVirtualMachine) {
// We're not initializing the JVM here -- but we could -- because most of the time
// we don't need the jvm. Instead we do the initialization on first use. This means we could get
// a nasty slow down the very first time, but we'll live with it for now.
}
~VirtualMachineEnv() {
if (mVirtualMachine) {
mVirtualMachine->DetachCurrentThread();
}
}
explicit VirtualMachineEnv(JavaVM* vm) noexcept;
~VirtualMachineEnv() noexcept;
JNIEnv* getEnvironmentSlow() noexcept;
static utils::Mutex sLock;
static JavaVM* sVirtualMachine;
static JavaVM* getVirtualMachine();
JNIEnv* mJniEnv = nullptr;
JavaVM* mVirtualMachine = nullptr;
};
filament/backend/src/VirtualMachineEnv.cpp
+81 -16
View File
@@ -18,12 +18,28 @@
#include <utils/compiler.h>
#include <utils/debug.h>
#include <utils/Mutex.h>
#include <utils/Panic.h>
#include <jni.h>
#include <mutex>
namespace filament {
JavaVM* VirtualMachineEnv::sVirtualMachine = nullptr;
using namespace utils;
// This Mutex shouldn't be subject to the Static Initialization Order Fiasco because its initial state is
// a single int initialized to 0.
/* static*/ Mutex VirtualMachineEnv::sLock;
/* static*/ JavaVM* VirtualMachineEnv::sVirtualMachine = nullptr;
UTILS_NOINLINE
JavaVM* VirtualMachineEnv::getVirtualMachine() {
std::lock_guard const lock(sLock);
assert_invariant(sVirtualMachine);
return sVirtualMachine;
}
/*
* This is typically called by filament_jni.so when it is loaded. If filament_jni.so is not used,
@@ -35,15 +51,75 @@ JavaVM* VirtualMachineEnv::sVirtualMachine = nullptr;
UTILS_PUBLIC
UTILS_NOINLINE
jint VirtualMachineEnv::JNI_OnLoad(JavaVM* vm) noexcept {
JNIEnv* env = nullptr;
if (UTILS_UNLIKELY(vm->GetEnv(reinterpret_cast<void**>(&env), JNI_VERSION_1_6) != JNI_OK)) {
// this should not happen
return -1;
std::lock_guard const lock(sLock);
if (sVirtualMachine) {
// It doesn't make sense for JNI_OnLoad() to be called more than once
return JNI_VERSION_1_6;
}
// Here we check this VM at least has JNI_VERSION_1_6
JNIEnv* env = nullptr;
jint const result = vm->GetEnv(reinterpret_cast<void**>(&env), JNI_VERSION_1_6);
FILAMENT_CHECK_POSTCONDITION(result == JNI_OK)
<< "Couldn't get JniEnv* from the VM, error = " << result;
sVirtualMachine = vm;
return JNI_VERSION_1_6;
}
UTILS_NOINLINE
VirtualMachineEnv& VirtualMachineEnv::get() noexcept {
JavaVM* const vm = getVirtualMachine();
// declaring this thread local, will ensure it's destroyed with the calling thread
thread_local VirtualMachineEnv instance{ vm };
return instance;
}
UTILS_NOINLINE
JNIEnv* VirtualMachineEnv::getThreadEnvironment() noexcept {
JavaVM* const vm = getVirtualMachine();
JNIEnv* env = nullptr;
jint const result = vm->GetEnv(reinterpret_cast<void**>(&env), JNI_VERSION_1_6);
FILAMENT_CHECK_POSTCONDITION(result == JNI_OK)
<< "Couldn't get JniEnv* from the VM, error = " << result;
return env;
}
VirtualMachineEnv::VirtualMachineEnv(JavaVM* vm) noexcept : mVirtualMachine(vm) {
// We're not initializing the JVM here -- but we could -- because most of the time
// we don't need the jvm. Instead, we do the initialization on first use. This means we could get
// a nasty slow down the very first time, but we'll live with it for now.
}
VirtualMachineEnv::~VirtualMachineEnv() noexcept {
if (mVirtualMachine) {
mVirtualMachine->DetachCurrentThread();
}
}
UTILS_NOINLINE
JNIEnv* VirtualMachineEnv::getEnvironmentSlow() noexcept {
FILAMENT_CHECK_PRECONDITION(mVirtualMachine)
<< "JNI_OnLoad() has not been called";
#if defined(__ANDROID__)
jint const result = mVirtualMachine->AttachCurrentThread(&mJniEnv, nullptr);
#else
jint const result = mVirtualMachine->AttachCurrentThread(reinterpret_cast<void**>(&mJniEnv), nullptr);
#endif
FILAMENT_CHECK_POSTCONDITION(result == JNI_OK)
<< "JavaVM::AttachCurrentThread failed with error " << result;
FILAMENT_CHECK_POSTCONDITION(mJniEnv)
<< "JavaVM::AttachCurrentThread returned a null mJniEnv";
return mJniEnv;
}
UTILS_NOINLINE
void VirtualMachineEnv::handleException(JNIEnv* const env) noexcept {
if (UTILS_UNLIKELY(env->ExceptionCheck())) {
@@ -52,16 +128,5 @@ void VirtualMachineEnv::handleException(JNIEnv* const env) noexcept {
}
}
UTILS_NOINLINE
JNIEnv* VirtualMachineEnv::getEnvironmentSlow() noexcept {
#if defined(__ANDROID__)
mVirtualMachine->AttachCurrentThread(&mJniEnv, nullptr);
#else
mVirtualMachine->AttachCurrentThread(reinterpret_cast<void**>(&mJniEnv), nullptr);
#endif
assert_invariant(mJniEnv);
return mJniEnv;
}
} // namespace filament
filament/backend/src/opengl/platforms/ExternalStreamManagerAndroid.cpp
+37 -36
View File
@@ -16,13 +16,28 @@
#include "ExternalStreamManagerAndroid.h"
#include <utils/api_level.h>
#include <private/backend/VirtualMachineEnv.h>
#include <backend/Platform.h>
#include <utils/compiler.h>
#include <utils/debug.h>
#include <utils/Log.h>
#include <utils/ostream.h>
#include <dlfcn.h>
#if __has_include(<android/surface_texture.h>)
# include <android/surface_texture.h>
# include <android/surface_texture_jni.h>
#else
struct ASurfaceTexture;
typedef struct ASurfaceTexture ASurfaceTexture;
#endif
#include <chrono>
#include <jni.h>
#include <stdint.h>
#include <new>
using namespace utils;
@@ -32,13 +47,8 @@ using namespace backend;
using Stream = Platform::Stream;
template <typename T>
static void loadSymbol(T*& pfn, const char *symbol) noexcept {
pfn = (T*)dlsym(RTLD_DEFAULT, symbol);
}
ExternalStreamManagerAndroid& ExternalStreamManagerAndroid::create() noexcept {
return *(new ExternalStreamManagerAndroid{});
return *(new(std::nothrow) ExternalStreamManagerAndroid{});
}
void ExternalStreamManagerAndroid::destroy(ExternalStreamManagerAndroid* pExternalStreamManagerAndroid) noexcept {
@@ -56,23 +66,13 @@ ExternalStreamManagerAndroid::~ExternalStreamManagerAndroid() noexcept = default
UTILS_NOINLINE
JNIEnv* ExternalStreamManagerAndroid::getEnvironmentSlow() noexcept {
JNIEnv * env = mVm.getEnvironment();
JNIEnv* const env = mVm.getEnvironment();
mJniEnv = env;
jclass SurfaceTextureClass = env->FindClass("android/graphics/SurfaceTexture");
mSurfaceTextureClass_updateTexImage = env->GetMethodID(
SurfaceTextureClass, "updateTexImage", "()V");
mSurfaceTextureClass_attachToGLContext = env->GetMethodID(
SurfaceTextureClass, "attachToGLContext", "(I)V");
mSurfaceTextureClass_detachFromGLContext = env->GetMethodID(
SurfaceTextureClass, "detachFromGLContext", "()V");
mSurfaceTextureClass_getTimestamp = env->GetMethodID(
SurfaceTextureClass, "getTimestamp", "()J");
mSurfaceTextureClass_updateTexImage = env->GetMethodID(SurfaceTextureClass, "updateTexImage", "()V");
mSurfaceTextureClass_attachToGLContext = env->GetMethodID(SurfaceTextureClass, "attachToGLContext", "(I)V");
mSurfaceTextureClass_detachFromGLContext = env->GetMethodID(SurfaceTextureClass, "detachFromGLContext", "()V");
mSurfaceTextureClass_getTimestamp = env->GetMethodID(SurfaceTextureClass, "getTimestamp", "()J");
return env;
}
@@ -82,7 +82,7 @@ Stream* ExternalStreamManagerAndroid::acquire(jobject surfaceTexture) noexcept {
if (!env) {
return nullptr; // this should not happen
}
EGLStream* stream = new EGLStream();
EGLStream* stream = new(std::nothrow) EGLStream();
stream->jSurfaceTexture = env->NewGlobalRef(surfaceTexture);
if (__builtin_available(android 28, *)) {
stream->nSurfaceTexture = ASurfaceTexture_fromSurfaceTexture(env, surfaceTexture);
@@ -91,29 +91,30 @@ Stream* ExternalStreamManagerAndroid::acquire(jobject surfaceTexture) noexcept {
}
void ExternalStreamManagerAndroid::release(Stream* handle) noexcept {
EGLStream* stream = static_cast<EGLStream*>(handle);
EGLStream const* stream = static_cast<EGLStream*>(handle);
if (__builtin_available(android 28, *)) {
ASurfaceTexture_release(stream->nSurfaceTexture);
}
JNIEnv* const env = getEnvironment();
// use VirtualMachineEnv::getEnvironment() directly because we don't need to cache JNI methods here
JNIEnv* const env = mVm.getEnvironment();
assert_invariant(env); // we should have called attach() by now
env->DeleteGlobalRef(stream->jSurfaceTexture);
delete stream;
}
void ExternalStreamManagerAndroid::attach(Stream* handle, intptr_t tname) noexcept {
EGLStream* stream = static_cast<EGLStream*>(handle);
EGLStream const* stream = static_cast<EGLStream*>(handle);
if (__builtin_available(android 28, *)) {
// associate our GL texture to the SurfaceTexture
ASurfaceTexture* const aSurfaceTexture = stream->nSurfaceTexture;
if (UTILS_UNLIKELY(ASurfaceTexture_attachToGLContext(aSurfaceTexture, (uint32_t)tname))) {
if (UTILS_UNLIKELY(ASurfaceTexture_attachToGLContext(aSurfaceTexture, uint32_t(tname)))) {
// Unfortunately, before API 26 SurfaceTexture was always created in attached mode,
// so attachToGLContext can fail. We consider this the unlikely case, because
// this is how it should work.
// So now we have to detach the surfacetexture from its texture
ASurfaceTexture_detachFromGLContext(aSurfaceTexture);
// and finally, try attaching again
ASurfaceTexture_attachToGLContext(aSurfaceTexture, (uint32_t)tname);
ASurfaceTexture_attachToGLContext(aSurfaceTexture, uint32_t(tname));
}
} else {
JNIEnv* const env = getEnvironment();
@@ -121,19 +122,19 @@ void ExternalStreamManagerAndroid::attach(Stream* handle, intptr_t tname) noexce
// associate our GL texture to the SurfaceTexture
jobject jSurfaceTexture = stream->jSurfaceTexture;
env->CallVoidMethod(jSurfaceTexture, mSurfaceTextureClass_attachToGLContext, (jint)tname);
env->CallVoidMethod(jSurfaceTexture, mSurfaceTextureClass_attachToGLContext, jint(tname));
if (UTILS_UNLIKELY(env->ExceptionCheck())) {
// Unfortunately, before API 26 SurfaceTexture was always created in attached mode,
// so attachToGLContext can fail. We consider this the unlikely case, because
// this is how it should work.
env->ExceptionClear();
// so now we have to detach the surfacetexture from its texture
// so now we have to detach the SurfaceTexture from its texture
env->CallVoidMethod(jSurfaceTexture, mSurfaceTextureClass_detachFromGLContext);
VirtualMachineEnv::handleException(env);
// and finally, try attaching again
env->CallVoidMethod(jSurfaceTexture, mSurfaceTextureClass_attachToGLContext, (jint)tname);
env->CallVoidMethod(jSurfaceTexture, mSurfaceTextureClass_attachToGLContext, jint(tname));
VirtualMachineEnv::handleException(env);
}
}
@@ -144,7 +145,7 @@ void ExternalStreamManagerAndroid::detach(Stream* handle) noexcept {
if (__builtin_available(android 28, *)) {
ASurfaceTexture_detachFromGLContext(stream->nSurfaceTexture);
} else {
JNIEnv* const env = mVm.getEnvironment();
JNIEnv* const env = getEnvironment();
assert_invariant(env); // we should have called attach() by now
env->CallVoidMethod(stream->jSurfaceTexture, mSurfaceTextureClass_detachFromGLContext);
VirtualMachineEnv::handleException(env);
@@ -152,12 +153,12 @@ void ExternalStreamManagerAndroid::detach(Stream* handle) noexcept {
}
void ExternalStreamManagerAndroid::updateTexImage(Stream* handle, int64_t* timestamp) noexcept {
EGLStream* stream = static_cast<EGLStream*>(handle);
EGLStream const* stream = static_cast<EGLStream*>(handle);
if (__builtin_available(android 28, *)) {
ASurfaceTexture_updateTexImage(stream->nSurfaceTexture);
*timestamp = ASurfaceTexture_getTimestamp(stream->nSurfaceTexture);
} else {
JNIEnv* const env = mVm.getEnvironment();
JNIEnv* const env = getEnvironment();
assert_invariant(env); // we should have called attach() by now
env->CallVoidMethod(stream->jSurfaceTexture, mSurfaceTextureClass_updateTexImage);
VirtualMachineEnv::handleException(env);
filament/backend/src/opengl/platforms/ExternalStreamManagerAndroid.h
+8 -3
View File
@@ -21,14 +21,19 @@
#include <backend/Platform.h>
#include <utils/compiler.h>
#if __has_include(<android/surface_texture.h>)
# include <android/surface_texture.h>
# include <android/surface_texture_jni.h>
#else
struct ASurfaceTexture;
typedef struct ASurfaceTexture ASurfaceTexture;
#endif
#include <jni.h>
#include <stdint.h>
namespace filament::backend {
/*
@@ -70,7 +75,8 @@ private:
ASurfaceTexture* nSurfaceTexture = nullptr;
};
inline JNIEnv* getEnvironment() noexcept {
// Must only be called from the backend thread
JNIEnv* getEnvironment() noexcept {
JNIEnv* env = mJniEnv;
if (UTILS_UNLIKELY(!env)) {
return getEnvironmentSlow();
@@ -85,7 +91,6 @@ private:
jmethodID mSurfaceTextureClass_attachToGLContext{};
jmethodID mSurfaceTextureClass_detachFromGLContext{};
};
} // namespace filament::backend
#endif //TNT_FILAMENT_BACKEND_OPENGL_ANDROID_EXTERNAL_STREAM_MANAGER_ANDROID_H