Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

attempt to fix crashers when initializing the JVM #8380

Merged
merged 1 commit into from
Feb 2, 2025
Merged

attempt to fix crashers when initializing the JVM #8380

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 14 additions & 28 deletions filament/backend/include/private/backend/VirtualMachineEnv.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,33 +18,25 @@
#define TNT_FILAMENT_DRIVER_ANDROID_VIRTUAL_MACHINE_ENV_H

#include <utils/compiler.h>
#include <utils/debug.h>
#include <utils/Mutex.h>

#include <jni.h>

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();
Expand All @@ -55,20 +47,14 @@ class VirtualMachineEnv {
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;
};
Expand Down
89 changes: 77 additions & 12 deletions filament/backend/src/VirtualMachineEnv.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -35,33 +51,82 @@ 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
void VirtualMachineEnv::handleException(JNIEnv* const env) noexcept {
if (UTILS_UNLIKELY(env->ExceptionCheck())) {
env->ExceptionDescribe();
env->ExceptionClear();
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__)
mVirtualMachine->AttachCurrentThread(&mJniEnv, nullptr);
jint const result = mVirtualMachine->AttachCurrentThread(&mJniEnv, nullptr);
#else
mVirtualMachine->AttachCurrentThread(reinterpret_cast<void**>(&mJniEnv), nullptr);
jint const result = mVirtualMachine->AttachCurrentThread(reinterpret_cast<void**>(&mJniEnv), nullptr);
#endif
assert_invariant(mJniEnv);

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())) {
env->ExceptionDescribe();
env->ExceptionClear();
}
}

} // namespace filament

73 changes: 37 additions & 36 deletions filament/backend/src/opengl/platforms/ExternalStreamManagerAndroid.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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>

#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 <dlfcn.h>
#include <jni.h>

#include <chrono>
#include <stdint.h>

#include <new>

using namespace utils;

Expand All @@ -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 {
Expand All @@ -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;
}

Expand All @@ -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);
Expand All @@ -91,49 +91,50 @@ 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();
assert_invariant(env); // we should have called attach() by now

// 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);
}
}
Expand All @@ -144,20 +145,20 @@ 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);
}
}

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);
Expand Down
11 changes: 8 additions & 3 deletions filament/backend/src/opengl/platforms/ExternalStreamManagerAndroid.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

/*
Expand Down Expand Up @@ -70,7 +75,8 @@ class ExternalStreamManagerAndroid {
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();
Expand All @@ -85,7 +91,6 @@ class ExternalStreamManagerAndroid {
jmethodID mSurfaceTextureClass_attachToGLContext{};
jmethodID mSurfaceTextureClass_detachFromGLContext{};
};

} // namespace filament::backend

#endif //TNT_FILAMENT_BACKEND_OPENGL_ANDROID_EXTERNAL_STREAM_MANAGER_ANDROID_H