Fix "using JNI after critical" exceptions
Refactored ScopedJavaNioBuffer implementation to copy directly to std::vector<uint8_t> instances and release the jni critical array queries immediately after copying into a vector. This is to avoid potential interleavings of JNI method calls before destructor of ScopedJavaNioBuffer is invoked. As per jni requirements no other JNI calls are allowed after GetPrimitiveArrayCritical until a matching call to ReleasePrimitiveArrayCritical is made. Fixes: 271468824 Test: Added tests to MeshTest Change-Id: Ia4afd8b8584ae43f021c79b6a341b05e80e0e205
This commit is contained in:
Nader Jawad
@@ -338,6 +338,7 @@ cc_defaults {
|
||||
"jni/android_util_PathParser.cpp",
|
||||
|
||||
"jni/Bitmap.cpp",
|
||||
"jni/BufferUtils.cpp",
|
||||
"jni/HardwareBufferHelpers.cpp",
|
||||
"jni/BitmapFactory.cpp",
|
||||
"jni/ByteBufferStreamAdaptor.cpp",
|
||||
|
||||
@@ -104,33 +104,31 @@ private:
|
||||
|
||||
class Mesh {
|
||||
public:
|
||||
Mesh(const sk_sp<SkMeshSpecification>& meshSpec, int mode, const void* vertexBuffer,
|
||||
size_t vertexBufferSize, jint vertexCount, jint vertexOffset,
|
||||
Mesh(const sk_sp<SkMeshSpecification>& meshSpec, int mode,
|
||||
std::vector<uint8_t>&& vertexBufferData, jint vertexCount, jint vertexOffset,
|
||||
std::unique_ptr<MeshUniformBuilder> builder, const SkRect& bounds)
|
||||
: mMeshSpec(meshSpec)
|
||||
, mMode(mode)
|
||||
, mVertexBufferData(std::move(vertexBufferData))
|
||||
, mVertexCount(vertexCount)
|
||||
, mVertexOffset(vertexOffset)
|
||||
, mBuilder(std::move(builder))
|
||||
, mBounds(bounds) {
|
||||
copyToVector(mVertexBufferData, vertexBuffer, vertexBufferSize);
|
||||
}
|
||||
, mBounds(bounds) {}
|
||||
|
||||
Mesh(const sk_sp<SkMeshSpecification>& meshSpec, int mode, const void* vertexBuffer,
|
||||
size_t vertexBufferSize, jint vertexCount, jint vertexOffset, const void* indexBuffer,
|
||||
size_t indexBufferSize, jint indexCount, jint indexOffset,
|
||||
Mesh(const sk_sp<SkMeshSpecification>& meshSpec, int mode,
|
||||
std::vector<uint8_t>&& vertexBufferData, jint vertexCount, jint vertexOffset,
|
||||
std::vector<uint8_t>&& indexBuffer, jint indexCount, jint indexOffset,
|
||||
std::unique_ptr<MeshUniformBuilder> builder, const SkRect& bounds)
|
||||
: mMeshSpec(meshSpec)
|
||||
, mMode(mode)
|
||||
, mVertexBufferData(std::move(vertexBufferData))
|
||||
, mVertexCount(vertexCount)
|
||||
, mVertexOffset(vertexOffset)
|
||||
, mIndexBufferData(std::move(indexBuffer))
|
||||
, mIndexCount(indexCount)
|
||||
, mIndexOffset(indexOffset)
|
||||
, mBuilder(std::move(builder))
|
||||
, mBounds(bounds) {
|
||||
copyToVector(mVertexBufferData, vertexBuffer, vertexBufferSize);
|
||||
copyToVector(mIndexBufferData, indexBuffer, indexBufferSize);
|
||||
}
|
||||
, mBounds(bounds) {}
|
||||
|
||||
Mesh(Mesh&&) = default;
|
||||
|
||||
@@ -180,13 +178,6 @@ public:
|
||||
MeshUniformBuilder* uniformBuilder() { return mBuilder.get(); }
|
||||
|
||||
private:
|
||||
void copyToVector(std::vector<uint8_t>& dst, const void* src, size_t srcSize) {
|
||||
if (src) {
|
||||
dst.resize(srcSize);
|
||||
memcpy(dst.data(), src, srcSize);
|
||||
}
|
||||
}
|
||||
|
||||
sk_sp<SkMeshSpecification> mMeshSpec;
|
||||
int mMode = 0;
|
||||
|
||||
|
||||
130
libs/hwui/jni/BufferUtils.cpp
Normal file
130
libs/hwui/jni/BufferUtils.cpp
Normal file
@@ -0,0 +1,130 @@
|
||||
/*
|
||||
* Copyright (C) 2023 The Android Open Source Project
|
||||
*
|
||||
* Licensed under the Apache License, Version 2.0 (the "License");
|
||||
* you may not use this file except in compliance with the License.
|
||||
* You may obtain a copy of the License at
|
||||
*
|
||||
* http://www.apache.org/licenses/LICENSE-2.0
|
||||
*
|
||||
* Unless required by applicable law or agreed to in writing, software
|
||||
* distributed under the License is distributed on an "AS IS" BASIS,
|
||||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
||||
* See the License for the specific language governing permissions and
|
||||
* limitations under the License.
|
||||
*/
|
||||
#include "BufferUtils.h"
|
||||
|
||||
#include "graphics_jni_helpers.h"
|
||||
|
||||
static void copyToVector(std::vector<uint8_t>& dst, const void* src, size_t srcSize) {
|
||||
if (src) {
|
||||
dst.resize(srcSize);
|
||||
memcpy(dst.data(), src, srcSize);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* This code is taken and modified from com_google_android_gles_jni_GLImpl.cpp to extract data
|
||||
* from a java.nio.Buffer.
|
||||
*/
|
||||
static void* getDirectBufferPointer(JNIEnv* env, jobject buffer) {
|
||||
if (buffer == nullptr) {
|
||||
return nullptr;
|
||||
}
|
||||
|
||||
jint position;
|
||||
jint limit;
|
||||
jint elementSizeShift;
|
||||
jlong pointer;
|
||||
pointer = jniGetNioBufferFields(env, buffer, &position, &limit, &elementSizeShift);
|
||||
if (pointer == 0) {
|
||||
jniThrowException(env, "java/lang/IllegalArgumentException",
|
||||
"Must use a native order direct Buffer");
|
||||
return nullptr;
|
||||
}
|
||||
pointer += position << elementSizeShift;
|
||||
return reinterpret_cast<void*>(pointer);
|
||||
}
|
||||
|
||||
static void releasePointer(JNIEnv* env, jarray array, void* data, jboolean commit) {
|
||||
env->ReleasePrimitiveArrayCritical(array, data, commit ? 0 : JNI_ABORT);
|
||||
}
|
||||
|
||||
static void* getPointer(JNIEnv* env, jobject buffer, jarray* array, jint* remaining, jint* offset) {
|
||||
jint position;
|
||||
jint limit;
|
||||
jint elementSizeShift;
|
||||
|
||||
jlong pointer;
|
||||
pointer = jniGetNioBufferFields(env, buffer, &position, &limit, &elementSizeShift);
|
||||
*remaining = (limit - position) << elementSizeShift;
|
||||
if (pointer != 0L) {
|
||||
*array = nullptr;
|
||||
pointer += position << elementSizeShift;
|
||||
return reinterpret_cast<void*>(pointer);
|
||||
}
|
||||
|
||||
*array = jniGetNioBufferBaseArray(env, buffer);
|
||||
*offset = jniGetNioBufferBaseArrayOffset(env, buffer);
|
||||
return nullptr;
|
||||
}
|
||||
|
||||
/**
|
||||
* This is a copy of
|
||||
* static void android_glBufferData__IILjava_nio_Buffer_2I
|
||||
* from com_google_android_gles_jni_GLImpl.cpp
|
||||
*/
|
||||
static void setIndirectData(JNIEnv* env, size_t size, jobject data_buf,
|
||||
std::vector<uint8_t>& result) {
|
||||
jint exception = 0;
|
||||
const char* exceptionType = nullptr;
|
||||
const char* exceptionMessage = nullptr;
|
||||
jarray array = nullptr;
|
||||
jint bufferOffset = 0;
|
||||
jint remaining;
|
||||
void* data = 0;
|
||||
char* dataBase = nullptr;
|
||||
|
||||
if (data_buf) {
|
||||
data = getPointer(env, data_buf, (jarray*)&array, &remaining, &bufferOffset);
|
||||
if (remaining < size) {
|
||||
exception = 1;
|
||||
exceptionType = "java/lang/IllegalArgumentException";
|
||||
exceptionMessage = "remaining() < size < needed";
|
||||
goto exit;
|
||||
}
|
||||
}
|
||||
if (data_buf && data == nullptr) {
|
||||
dataBase = (char*)env->GetPrimitiveArrayCritical(array, (jboolean*)0);
|
||||
data = (void*)(dataBase + bufferOffset);
|
||||
}
|
||||
|
||||
copyToVector(result, data, size);
|
||||
|
||||
exit:
|
||||
if (array) {
|
||||
releasePointer(env, array, (void*)dataBase, JNI_FALSE);
|
||||
}
|
||||
if (exception) {
|
||||
jniThrowException(env, exceptionType, exceptionMessage);
|
||||
}
|
||||
}
|
||||
|
||||
std::vector<uint8_t> copyJavaNioBufferToVector(JNIEnv* env, jobject buffer, size_t size,
|
||||
jboolean isDirect) {
|
||||
std::vector<uint8_t> data;
|
||||
if (buffer == nullptr) {
|
||||
jniThrowNullPointerException(env);
|
||||
} else {
|
||||
if (isDirect) {
|
||||
void* directBufferPtr = getDirectBufferPointer(env, buffer);
|
||||
if (directBufferPtr) {
|
||||
copyToVector(data, directBufferPtr, size);
|
||||
}
|
||||
} else {
|
||||
setIndirectData(env, size, buffer, data);
|
||||
}
|
||||
}
|
||||
return data;
|
||||
}
|
||||
32
libs/hwui/jni/BufferUtils.h
Normal file
32
libs/hwui/jni/BufferUtils.h
Normal file
@@ -0,0 +1,32 @@
|
||||
/*
|
||||
* Copyright (C) 2023 The Android Open Source Project
|
||||
*
|
||||
* Licensed under the Apache License, Version 2.0 (the "License");
|
||||
* you may not use this file except in compliance with the License.
|
||||
* You may obtain a copy of the License at
|
||||
*
|
||||
* http://www.apache.org/licenses/LICENSE-2.0
|
||||
*
|
||||
* Unless required by applicable law or agreed to in writing, software
|
||||
* distributed under the License is distributed on an "AS IS" BASIS,
|
||||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
||||
* See the License for the specific language governing permissions and
|
||||
* limitations under the License.
|
||||
*/
|
||||
#ifndef BUFFERUTILS_H_
|
||||
#define BUFFERUTILS_H_
|
||||
|
||||
#include <jni.h>
|
||||
|
||||
#include <vector>
|
||||
|
||||
/**
|
||||
* Helper method to load a java.nio.Buffer instance into a vector. This handles
|
||||
* both direct and indirect buffers and promptly releases any critical arrays that
|
||||
* have been retrieved in order to avoid potential jni exceptions due to interleaved
|
||||
* jni calls between get/release primitive method invocations.
|
||||
*/
|
||||
std::vector<uint8_t> copyJavaNioBufferToVector(JNIEnv* env, jobject buffer, size_t size,
|
||||
jboolean isDirect);
|
||||
|
||||
#endif // BUFFERUTILS_H_
|
||||
@@ -14,172 +14,18 @@
|
||||
* limitations under the License.
|
||||
*/
|
||||
|
||||
#include <GrDirectContext.h>
|
||||
#include <Mesh.h>
|
||||
#include <SkMesh.h>
|
||||
#include <jni.h>
|
||||
#include <log/log.h>
|
||||
|
||||
#include <utility>
|
||||
|
||||
#include "BufferUtils.h"
|
||||
#include "GraphicsJNI.h"
|
||||
#include "graphics_jni_helpers.h"
|
||||
|
||||
#define gIndexByteSize 2
|
||||
|
||||
// A smart pointer that provides read only access to Java.nio.Buffer. This handles both
|
||||
// direct and indrect buffers, allowing access to the underlying data in both
|
||||
// situations. If passed a null buffer, we will throw NullPointerException,
|
||||
// and c_data will return nullptr.
|
||||
//
|
||||
// This class draws from com_google_android_gles_jni_GLImpl.cpp for Buffer to void *
|
||||
// conversion.
|
||||
class ScopedJavaNioBuffer {
|
||||
public:
|
||||
ScopedJavaNioBuffer(JNIEnv* env, jobject buffer, size_t size, jboolean isDirect)
|
||||
: mEnv(env), mBuffer(buffer) {
|
||||
if (buffer == nullptr) {
|
||||
mDataBase = nullptr;
|
||||
mData = nullptr;
|
||||
jniThrowNullPointerException(env);
|
||||
} else {
|
||||
mArray = (jarray) nullptr;
|
||||
if (isDirect) {
|
||||
mData = getDirectBufferPointer(mEnv, mBuffer);
|
||||
} else {
|
||||
mData = setIndirectData(size);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
ScopedJavaNioBuffer(ScopedJavaNioBuffer&& rhs) noexcept { *this = std::move(rhs); }
|
||||
|
||||
~ScopedJavaNioBuffer() { reset(); }
|
||||
|
||||
void reset() {
|
||||
if (mDataBase) {
|
||||
releasePointer(mEnv, mArray, mDataBase, JNI_FALSE);
|
||||
mDataBase = nullptr;
|
||||
}
|
||||
}
|
||||
|
||||
ScopedJavaNioBuffer& operator=(ScopedJavaNioBuffer&& rhs) noexcept {
|
||||
if (this != &rhs) {
|
||||
reset();
|
||||
|
||||
mEnv = rhs.mEnv;
|
||||
mBuffer = rhs.mBuffer;
|
||||
mDataBase = rhs.mDataBase;
|
||||
mData = rhs.mData;
|
||||
mArray = rhs.mArray;
|
||||
rhs.mEnv = nullptr;
|
||||
rhs.mData = nullptr;
|
||||
rhs.mBuffer = nullptr;
|
||||
rhs.mArray = nullptr;
|
||||
rhs.mDataBase = nullptr;
|
||||
}
|
||||
return *this;
|
||||
}
|
||||
|
||||
const void* data() const { return mData; }
|
||||
|
||||
private:
|
||||
/**
|
||||
* This code is taken and modified from com_google_android_gles_jni_GLImpl.cpp to extract data
|
||||
* from a java.nio.Buffer.
|
||||
*/
|
||||
void* getDirectBufferPointer(JNIEnv* env, jobject buffer) {
|
||||
if (buffer == nullptr) {
|
||||
return nullptr;
|
||||
}
|
||||
|
||||
jint position;
|
||||
jint limit;
|
||||
jint elementSizeShift;
|
||||
jlong pointer;
|
||||
pointer = jniGetNioBufferFields(env, buffer, &position, &limit, &elementSizeShift);
|
||||
if (pointer == 0) {
|
||||
jniThrowException(mEnv, "java/lang/IllegalArgumentException",
|
||||
"Must use a native order direct Buffer");
|
||||
return nullptr;
|
||||
}
|
||||
pointer += position << elementSizeShift;
|
||||
return reinterpret_cast<void*>(pointer);
|
||||
}
|
||||
|
||||
static void releasePointer(JNIEnv* env, jarray array, void* data, jboolean commit) {
|
||||
env->ReleasePrimitiveArrayCritical(array, data, commit ? 0 : JNI_ABORT);
|
||||
}
|
||||
|
||||
static void* getPointer(JNIEnv* env, jobject buffer, jarray* array, jint* remaining,
|
||||
jint* offset) {
|
||||
jint position;
|
||||
jint limit;
|
||||
jint elementSizeShift;
|
||||
|
||||
jlong pointer;
|
||||
pointer = jniGetNioBufferFields(env, buffer, &position, &limit, &elementSizeShift);
|
||||
*remaining = (limit - position) << elementSizeShift;
|
||||
if (pointer != 0L) {
|
||||
*array = nullptr;
|
||||
pointer += position << elementSizeShift;
|
||||
return reinterpret_cast<void*>(pointer);
|
||||
}
|
||||
|
||||
*array = jniGetNioBufferBaseArray(env, buffer);
|
||||
*offset = jniGetNioBufferBaseArrayOffset(env, buffer);
|
||||
return nullptr;
|
||||
}
|
||||
|
||||
/**
|
||||
* This is a copy of
|
||||
* static void android_glBufferData__IILjava_nio_Buffer_2I
|
||||
* from com_google_android_gles_jni_GLImpl.cpp
|
||||
*/
|
||||
void* setIndirectData(size_t size) {
|
||||
jint exception;
|
||||
const char* exceptionType;
|
||||
const char* exceptionMessage;
|
||||
jint bufferOffset = (jint)0;
|
||||
jint remaining;
|
||||
void* tempData;
|
||||
|
||||
if (mBuffer) {
|
||||
tempData =
|
||||
(void*)getPointer(mEnv, mBuffer, (jarray*)&mArray, &remaining, &bufferOffset);
|
||||
if (remaining < size) {
|
||||
exception = 1;
|
||||
exceptionType = "java/lang/IllegalArgumentException";
|
||||
exceptionMessage = "remaining() < size < needed";
|
||||
goto exit;
|
||||
}
|
||||
}
|
||||
if (mBuffer && tempData == nullptr) {
|
||||
mDataBase = (char*)mEnv->GetPrimitiveArrayCritical(mArray, (jboolean*)0);
|
||||
tempData = (void*)(mDataBase + bufferOffset);
|
||||
}
|
||||
return tempData;
|
||||
exit:
|
||||
if (mArray) {
|
||||
releasePointer(mEnv, mArray, (void*)(mDataBase), JNI_FALSE);
|
||||
}
|
||||
if (exception) {
|
||||
jniThrowException(mEnv, exceptionType, exceptionMessage);
|
||||
}
|
||||
return nullptr;
|
||||
}
|
||||
|
||||
JNIEnv* mEnv;
|
||||
|
||||
// Java Buffer data
|
||||
void* mData;
|
||||
jobject mBuffer;
|
||||
|
||||
// Indirect Buffer Data
|
||||
jarray mArray;
|
||||
char* mDataBase;
|
||||
};
|
||||
|
||||
namespace android {
|
||||
|
||||
static jlong make(JNIEnv* env, jobject, jlong meshSpec, jint mode, jobject vertexBuffer,
|
||||
@@ -187,9 +33,12 @@ static jlong make(JNIEnv* env, jobject, jlong meshSpec, jint mode, jobject verte
|
||||
jfloat right, jfloat bottom) {
|
||||
auto skMeshSpec = sk_ref_sp(reinterpret_cast<SkMeshSpecification*>(meshSpec));
|
||||
size_t bufferSize = vertexCount * skMeshSpec->stride();
|
||||
auto buff = ScopedJavaNioBuffer(env, vertexBuffer, bufferSize, isDirect);
|
||||
auto buffer = copyJavaNioBufferToVector(env, vertexBuffer, bufferSize, isDirect);
|
||||
if (env->ExceptionCheck()) {
|
||||
return 0;
|
||||
}
|
||||
auto skRect = SkRect::MakeLTRB(left, top, right, bottom);
|
||||
auto meshPtr = new Mesh(skMeshSpec, mode, buff.data(), bufferSize, vertexCount, vertexOffset,
|
||||
auto meshPtr = new Mesh(skMeshSpec, mode, std::move(buffer), vertexCount, vertexOffset,
|
||||
std::make_unique<MeshUniformBuilder>(skMeshSpec), skRect);
|
||||
auto [valid, msg] = meshPtr->validate();
|
||||
if (!valid) {
|
||||
@@ -205,11 +54,17 @@ static jlong makeIndexed(JNIEnv* env, jobject, jlong meshSpec, jint mode, jobjec
|
||||
auto skMeshSpec = sk_ref_sp(reinterpret_cast<SkMeshSpecification*>(meshSpec));
|
||||
auto vertexBufferSize = vertexCount * skMeshSpec->stride();
|
||||
auto indexBufferSize = indexCount * gIndexByteSize;
|
||||
auto vBuf = ScopedJavaNioBuffer(env, vertexBuffer, vertexBufferSize, isVertexDirect);
|
||||
auto iBuf = ScopedJavaNioBuffer(env, indexBuffer, indexBufferSize, isIndexDirect);
|
||||
auto vBuf = copyJavaNioBufferToVector(env, vertexBuffer, vertexBufferSize, isVertexDirect);
|
||||
if (env->ExceptionCheck()) {
|
||||
return 0;
|
||||
}
|
||||
auto iBuf = copyJavaNioBufferToVector(env, indexBuffer, indexBufferSize, isIndexDirect);
|
||||
if (env->ExceptionCheck()) {
|
||||
return 0;
|
||||
}
|
||||
auto skRect = SkRect::MakeLTRB(left, top, right, bottom);
|
||||
auto meshPtr = new Mesh(skMeshSpec, mode, vBuf.data(), vertexBufferSize, vertexCount,
|
||||
vertexOffset, iBuf.data(), indexBufferSize, indexCount, indexOffset,
|
||||
auto meshPtr = new Mesh(skMeshSpec, mode, std::move(vBuf), vertexCount, vertexOffset,
|
||||
std::move(iBuf), indexCount, indexOffset,
|
||||
std::make_unique<MeshUniformBuilder>(skMeshSpec), skRect);
|
||||
auto [valid, msg] = meshPtr->validate();
|
||||
if (!valid) {
|
||||
|
||||
Reference in New Issue
Block a user