From 0723cc0772a5e91d5244f097bec2e49e983ab4b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roman=20R=C3=A4dle?= Date: Mon, 12 Dec 2022 09:55:48 -0800 Subject: [PATCH] Fix issue with JNI Env not available (#177) Summary: Pull Request resolved: https://github.com/facebookresearch/playtorch/pull/177 The Hermes GC will eventually clean up orphan host objects. This is also the case for `ImageHostObject`, which itself has a reference to `Image` that needs to be cleaned up, hence the call to the `Image::~Image` destructor. On Android, it will try to release the `global_ref` resource, which at the time, may not run in the JVM thread scope causing an app carsh with an error like: ``` Abort message: 'terminating with uncaught exception of type std::runtime_error: Unable to retrieve jni environment. Is the thread attached?' ``` ``` /data/app/~~tGo_W1LgnykvwiT9t7jBtQ==/dev.playtorch-NUv9YPDoqWoQiv-dKR2ylA==/lib/arm64/libtorchlive.so (__clang_call_terminate+8) (BuildId: 0a803d5f698a12e07d4c31b32095ee75a8eefd87) 12-10 18:51:24.290 20638 20638 F DEBUG : #6 pc 0000000000088f00 /data/app/~~tGo_W1LgnykvwiT9t7jBtQ==/dev.playtorch-NUv9YPDoqWoQiv-dKR2ylA==/lib/arm64/libtorchlive.so (torchlive::media::Image::~Image()+92) (BuildId: 0a803d5f698a12e07d4c31b32095ee75a8eefd87) 12-10 18:51:24.290 20638 20638 F DEBUG : #7 pc 0000000000043874 /data/app/~~tGo_W1LgnykvwiT9t7jBtQ==/dev.playtorch-NUv9YPDoqWoQiv-dKR2ylA==/lib/arm64/libtorchlive.so (torchlive::media::ImageHostObject::~ImageHostObject()+76) (BuildId: 0a803d5f698a12e07d4c31b32095ee75a8eefd87) 12-10 18:51:24.290 20638 20638 F DEBUG : #8 pc 000000000001cc20 /data/app/~~tGo_W1LgnykvwiT9t7jBtQ==/dev.playtorch-NUv9YPDoqWoQiv-dKR2ylA==/lib/arm64/libhermes-executor-release.so (facebook::jsi::DecoratedHostObject::~DecoratedHostObject()+76) (BuildId: f9ec095fd26bd03ab3b56d56ec655670c546c472) 12-10 18:51:24.290 20638 20638 F DEBUG : #9 pc 000000000007033c /data/app/~~tGo_W1LgnykvwiT9t7jBtQ==/dev.playtorch-NUv9YPDoqWoQiv-dKR2ylA==/lib/arm64/libhermes.so (BuildId: 957ec0f39e08feadc6d0ec99bbe00fa14a3687b3) ``` Reported here: https://github.com/facebookresearch/playtorch/issues/175 This change ensures that the image instance release happens within the JVM environment. More details: https://github.com/facebookincubator/fbjni/blob/5eacdd11c5d9c46f5a752faad3457f3b07fe40cb/cxx/fbjni/detail/Environment.h#L114 Reviewed By: ansonsyfang Differential Revision: D41919866 fbshipit-source-id: 103fb2d5bdadf42e8d79882874147abe58d3a1c2 --- .../android/src/main/cpp/torchlive/media/image/Image.cpp | 7 +++++++ .../android/src/main/cpp/torchlive/media/image/Image.h | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/react-native-pytorch-core/android/src/main/cpp/torchlive/media/image/Image.cpp b/react-native-pytorch-core/android/src/main/cpp/torchlive/media/image/Image.cpp index afd08945b..4e5c42e4b 100644 --- a/react-native-pytorch-core/android/src/main/cpp/torchlive/media/image/Image.cpp +++ b/react-native-pytorch-core/android/src/main/cpp/torchlive/media/image/Image.cpp @@ -33,6 +33,13 @@ Image::Image(facebook::jni::alias_ref image) id_ = wrapObjectMethod(mediaUtilsClass, image)->toStdString(); } +Image::~Image() { + ThreadScope::WithClassLoader([&]() { + Environment::ensureCurrentThreadIsAttached(); + this->image_.release(); + }); +} + std::string Image::getId() const { return id_; } diff --git a/react-native-pytorch-core/android/src/main/cpp/torchlive/media/image/Image.h b/react-native-pytorch-core/android/src/main/cpp/torchlive/media/image/Image.h index 42b005f93..c063e4052 100644 --- a/react-native-pytorch-core/android/src/main/cpp/torchlive/media/image/Image.h +++ b/react-native-pytorch-core/android/src/main/cpp/torchlive/media/image/Image.h @@ -19,7 +19,7 @@ namespace media { class Image : public IImage { public: Image(facebook::jni::alias_ref image); - ~Image() override = default; + ~Image() override; std::string getId() const override;