Make exceptions thrown by JNI calls to Image.getHardwareBuffer non-fatal#185125
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a JNI mocking framework for Android platform views and adds unit tests for PlatformViewAndroidJNIImpl. It updates ImageGetHardwareBuffer to gracefully handle JNI exceptions by clearing them and returning an empty reference, and adds null checks for hardware buffers in the OpenGL and Vulkan external texture implementations. Review feedback identifies a dangling pointer risk in the test suite's JVM setup and suggests completing the JNI function table assignments in the mock environment.
| MockJNIEnv mock_env; | ||
| SetJNIEnv(&mock_env); |
There was a problem hiding this comment.
mock_env is a local variable within SetUpJVM. Since SetJNIEnv stores its address in the static jvm_ instance, this pointer becomes dangling as soon as SetUpJVM returns. This can lead to undefined behavior or crashes if any code attempts to access the JNI environment via the VM before the next test sets a valid environment.
| MockJNIEnv mock_env; | |
| SetJNIEnv(&mock_env); | |
| static MockJNIEnv mock_env; | |
| SetJNIEnv(&mock_env); |
There was a problem hiding this comment.
Added an RAII wrapper for SetJNIEnv that nulls the mock JVM's environment when the test exits
| // Replace the JNIEnv's function table with wrappers that invoke the | ||
| // mockable virtual methods in this class. | ||
| functions = &jni_; | ||
| jni_.CallObjectMethodV = WrapCallObjectMethodV; |
There was a problem hiding this comment.
The WrapCallObjectMethod function is defined (lines 90-99) but not assigned to the jni_ function table in the constructor. While C++ JNIEnv helpers often delegate to CallObjectMethodV, it is safer and more complete to hook up both to ensure the mock behaves correctly regardless of how the JNI call is invoked.
| jni_.CallObjectMethodV = WrapCallObjectMethodV; | |
| jni_.CallObjectMethod = WrapCallObjectMethod; | |
| jni_.CallObjectMethodV = WrapCallObjectMethodV; |
If a call to Android's Image.getHardwareBuffer API made by PlatformViewAndroidJNIImpl::ImageGetHardwareBuffer throws an exception, then the ImageExternalTexture can skip rendering of this frame. It is excessive to make this a fatal exception that terminates the process. Fixes flutter#175267 Fixes flutter#180804
2be2d56 to
4ab8ad9
Compare
Thanks a lot for resolving this issue promptly. I’d like to inquire when this fix is expected to be merged into the stable release and rolled out via the upgrade update. |
|
To determine what versions of Flutter contain the fix, please see where’s my commit. If the commit has no release tag, it is currently only available in the master channel. |
If a call to Android's Image.getHardwareBuffer API made by PlatformViewAndroidJNIImpl::ImageGetHardwareBuffer throws an exception, then the ImageExternalTexture can skip rendering of this frame. It is excessive to make this a fatal exception that terminates the process.
Fixes #175267
Fixes #180804