DaddyFrosty/yoga
1
0
Fork 0
Code Issues 96 Pull Requests 31 Actions Packages Projects Releases 33 Wiki Activity

Fix usage of weak references to check for null

Browse Source 
Summary: In case the java object has been GCed during layout calculation we will crash if we try to call a method on the now invalid weak pointer. Use fbjni weak_ref to skip calling to java when the java object does not exist any more.

Reviewed By: lexs

Differential Revision: D4251133

fbshipit-source-id: 2d8949252b31447ce54bc16a35cb25fabe72230b
This commit is contained in:
Emil Sjolander
2016-11-30 10:07:53 -08:00
committed by Facebook Github Bot
parent 4fbe0495b4
commit c5bbcd78ae
1 changed files with 45 additions and 33 deletions
Download Patch File Download Diff File Expand all files Collapse all files

78
java/jni/CSSJNI.cpp
View File

@@ -14,35 +14,42 @@
using namespace facebook::jni; using namespace facebook::jni;
using namespace std; using namespace std;
static inline weak_ref<jobject> *jobjectContext(CSSNodeRef node) {
return reinterpret_cast<weak_ref<jobject> *>(CSSNodeGetContext(node));
}
static void _jniTransferLayoutDirection(CSSNodeRef node, alias_ref<jobject> javaNode) { static void _jniTransferLayoutDirection(CSSNodeRef node, alias_ref<jobject> javaNode) {
static auto layoutDirectionField = javaNode->getClass()->getField<jint>("mLayoutDirection"); static auto layoutDirectionField = javaNode->getClass()->getField<jint>("mLayoutDirection");
javaNode->setFieldValue(layoutDirectionField, static_cast<jint>(CSSNodeLayoutGetDirection(node))); javaNode->setFieldValue(layoutDirectionField, static_cast<jint>(CSSNodeLayoutGetDirection(node)));
} }
static void _jniTransferLayoutOutputsRecursive(CSSNodeRef root) { static void _jniTransferLayoutOutputsRecursive(CSSNodeRef root) {
auto javaNode = adopt_local( if (auto obj = jobjectContext(root)->lockLocal()) {
Environment::current()->NewLocalRef(reinterpret_cast<jweak>(CSSNodeGetContext(root)))); static auto widthField = obj->getClass()->getField<jfloat>("mWidth");
static auto heightField = obj->getClass()->getField<jfloat>("mHeight");
static auto leftField = obj->getClass()->getField<jfloat>("mLeft");
static auto topField = obj->getClass()->getField<jfloat>("mTop");
static auto widthField = javaNode->getClass()->getField<jfloat>("mWidth"); obj->setFieldValue(widthField, CSSNodeLayoutGetWidth(root));
static auto heightField = javaNode->getClass()->getField<jfloat>("mHeight"); obj->setFieldValue(heightField, CSSNodeLayoutGetHeight(root));
static auto leftField = javaNode->getClass()->getField<jfloat>("mLeft"); obj->setFieldValue(leftField, CSSNodeLayoutGetLeft(root));
static auto topField = javaNode->getClass()->getField<jfloat>("mTop"); obj->setFieldValue(topField, CSSNodeLayoutGetTop(root));
_jniTransferLayoutDirection(root, obj);
javaNode->setFieldValue(widthField, CSSNodeLayoutGetWidth(root)); for (uint32_t i = 0; i < CSSNodeChildCount(root); i++) {
javaNode->setFieldValue(heightField, CSSNodeLayoutGetHeight(root)); _jniTransferLayoutOutputsRecursive(CSSNodeGetChild(root, i));
javaNode->setFieldValue(leftField, CSSNodeLayoutGetLeft(root)); }
javaNode->setFieldValue(topField, CSSNodeLayoutGetTop(root)); } else {
_jniTransferLayoutDirection(root, javaNode); CSSLog(CSSLogLevelError, "Java CSSNode was GCed during layout calculation\n");
for (uint32_t i = 0; i < CSSNodeChildCount(root); i++) {
_jniTransferLayoutOutputsRecursive(CSSNodeGetChild(root, i));
} }
} }
static void _jniPrint(CSSNodeRef node) { static void _jniPrint(CSSNodeRef node) {
auto obj = adopt_local( if (auto obj = jobjectContext(node)->lockLocal()) {
Environment::current()->NewLocalRef(reinterpret_cast<jweak>(CSSNodeGetContext(node)))); cout << obj->toString() << endl;
cout << obj->toString() << endl; } else {
CSSLog(CSSLogLevelError, "Java CSSNode was GCed during layout calculation\n");
}
} }
static CSSSize _jniMeasureFunc(CSSNodeRef node, static CSSSize _jniMeasureFunc(CSSNodeRef node,
@@ -50,22 +57,27 @@ static CSSSize _jniMeasureFunc(CSSNodeRef node,
CSSMeasureMode widthMode, CSSMeasureMode widthMode,
float height, float height,
CSSMeasureMode heightMode) { CSSMeasureMode heightMode) {
auto obj = adopt_local( if (auto obj = jobjectContext(node)->lockLocal()) {
Environment::current()->NewLocalRef(reinterpret_cast<jweak>(CSSNodeGetContext(node)))); static auto measureFunc = findClassLocal("com/facebook/csslayout/CSSNode")
->getMethod<jlong(jfloat, jint, jfloat, jint)>("measure");
static auto measureFunc = findClassLocal("com/facebook/csslayout/CSSNode") _jniTransferLayoutDirection(node, obj);
->getMethod<jlong(jfloat, jint, jfloat, jint)>("measure"); const auto measureResult = measureFunc(obj, width, widthMode, height, heightMode);
_jniTransferLayoutDirection(node, obj); static_assert(sizeof(measureResult) == 8,
const auto measureResult = measureFunc(obj, width, widthMode, height, heightMode); "Expected measureResult to be 8 bytes, or two 32 bit ints");
static_assert(sizeof(measureResult) == 8, const float measuredWidth = static_cast<float>(0xFFFFFFFF & (measureResult >> 32));
"Expected measureResult to be 8 bytes, or two 32 bit ints"); const float measuredHeight = static_cast<float>(0xFFFFFFFF & measureResult);
const float measuredWidth = static_cast<float>(0xFFFFFFFF & (measureResult >> 32)); return CSSSize{measuredWidth, measuredHeight};
const float measuredHeight = static_cast<float>(0xFFFFFFFF & measureResult); } else {
CSSLog(CSSLogLevelError, "Java CSSNode was GCed during layout calculation\n");
return CSSSize{measuredWidth, measuredHeight}; return CSSSize{
widthMode == CSSMeasureModeUndefined ? 0 : width,
heightMode == CSSMeasureModeUndefined ? 0 : height,
};
}
} }
struct JCSSLogLevel : public JavaClass<JCSSLogLevel> { struct JCSSLogLevel : public JavaClass<JCSSLogLevel> {
@@ -131,15 +143,15 @@ jint jni_CSSNodeGetInstanceCount(alias_ref<jclass> clazz) {
jlong jni_CSSNodeNew(alias_ref<jobject> thiz) { jlong jni_CSSNodeNew(alias_ref<jobject> thiz) {
const CSSNodeRef node = CSSNodeNew(); const CSSNodeRef node = CSSNodeNew();
CSSNodeSetContext(node, Environment::current()->NewWeakGlobalRef(thiz.get())); CSSNodeSetContext(node, new weak_ref<jobject>(make_weak(thiz)));
CSSNodeSetPrintFunc(node, _jniPrint); CSSNodeSetPrintFunc(node, _jniPrint);
return reinterpret_cast<jlong>(node); return reinterpret_cast<jlong>(node);
} }
void jni_CSSNodeFree(alias_ref<jobject> thiz, jlong nativePointer) { void jni_CSSNodeFree(alias_ref<jobject> thiz, jlong nativePointer) {
Environment::current()->DeleteWeakGlobalRef( const CSSNodeRef node = _jlong2CSSNodeRef(nativePointer);
reinterpret_cast<jweak>(CSSNodeGetContext(_jlong2CSSNodeRef(nativePointer)))); delete jobjectContext(node);
CSSNodeFree(_jlong2CSSNodeRef(nativePointer)); CSSNodeFree(node);
} }
void jni_CSSNodeReset(alias_ref<jobject> thiz, jlong nativePointer) { void jni_CSSNodeReset(alias_ref<jobject> thiz, jlong nativePointer) {