From 4b59360a59a8eb83bd6878559ef2b7a5392a8b10 Mon Sep 17 00:00:00 2001 From: Joshua Gross Date: Fri, 13 Sep 2019 17:50:25 -0700 Subject: [PATCH] Fix memory leak in experiment: JNI leak Summary: I think this array is copied when we call the function over the JNI, and that we need to free the local copy we make. Reviewed By: mdvacca Differential Revision: D17377077 fbshipit-source-id: 82fe4ec89e95335a329f4ce562441561dbe88693 --- .../java/com/facebook/react/fabric/jni/Binding.cpp | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/Binding.cpp b/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/Binding.cpp index ae5a9b3fa36..57180fb4790 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/Binding.cpp +++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/Binding.cpp @@ -489,7 +489,16 @@ local_ref createRemoveAndDeleteMultiMountItem( jni::findClassStatic(UIManagerJavaDescriptor) ->getMethod(jintArray)>("removeDeleteMultiMountItem"); - return removeDeleteMultiInstruction(javaUIManager, removeAndDeleteArray); + auto ret = removeDeleteMultiInstruction(javaUIManager, removeAndDeleteArray); + + // It is not strictly necessary to manually delete the ref here, in this particular case. + // If JNI memory is being allocated in a loop, it's easy to overload the localref table + // and crash; this is not possible in this case since the JNI would automatically clear this + // ref when it goes out of scope, anyway. However, this is being left here as a reminder of + // good hygiene and to be careful with JNI-allocated memory in general. + env->DeleteLocalRef(removeAndDeleteArray); + + return ret; } // TODO T48019320: because we pass initial props and state to the Create (and preallocate) mount instruction,