Skip to content
This repository has been archived by the owner on Oct 23, 2023. It is now read-only.

Commit

Permalink
Fix issue with JNI Env not available (#177)
Browse files Browse the repository at this point in the history
Summary:
Pull Request resolved: #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<JImage>` 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: #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
  • Loading branch information
raedle authored and facebook-github-bot committed Dec 12, 2022
1 parent d424ec3 commit 623f0f5
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,13 @@ Image::Image(facebook::jni::alias_ref<JIImage> image)
id_ = wrapObjectMethod(mediaUtilsClass, image)->toStdString();
}

Image::~Image() {
ThreadScope::WithClassLoader([&]() {
Environment::ensureCurrentThreadIsAttached();
this->image_.release();
});
}

std::string Image::getId() const {
return id_;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ namespace media {
class Image : public IImage {
public:
Image(facebook::jni::alias_ref<JIImage> image);
~Image() override = default;
~Image() override;

std::string getId() const override;

Expand Down

0 comments on commit 623f0f5

Please sign in to comment.