zqz/platform-external-webrtc
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix a bug in video_encoder_wrapper where int array was not freed properly.

Browse Source 
JNI_COMMIT doesn't actually free the buffer.

From JNI docs:
0: copy back the content and free the elems buffer
JNI_COMMIT: copy back the content but do not free the elems buffer
JNI_ABORT: free the buffer without copying back the possible changes

Also introduces helper methods to help avoid this problem in the
future.

Bug: webrtc:10132
Change-Id: I769df286d3bd186fdf39ee2363e9002f36454509
Reviewed-on: https://webrtc-review.googlesource.com/c/120600
Reviewed-by: Magnus Jedvert <magjed@webrtc.org>
Commit-Queue: Sami Kalliomäki <sakal@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#26529}
This commit is contained in:
Sami Kalliomäki
2019-02-04 13:42:11 +01:00
committed by Commit Bot
parent eee110dea2
commit ee61f9440a
7 changed files with 110 additions and 21 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
sdk/android/BUILD.gn
View File

@ -860,6 +860,7 @@ if (is_android) {
":generated_external_classes_jni",
":generated_native_api_jni",
":internal_jni",
"//api:array_view",
"//rtc_base:checks",
"//rtc_base:rtc_base_approved",
"//third_party/abseil-cpp/absl/types:optional",

43
sdk/android/native_api/jni/java_types.cc
View File

@ -206,6 +206,49 @@ ScopedJavaLocalRef<jstring> NativeToJavaString(
return str ? NativeToJavaString(jni, *str) : nullptr;
}
ScopedJavaLocalRef<jbyteArray> NativeToJavaByteArray(
JNIEnv* env,
rtc::ArrayView<int8_t> container) {
ScopedJavaLocalRef<jbyteArray> jarray(env,
env->NewByteArray(container.size()));
int8_t* array_ptr =
env->GetByteArrayElements(jarray.obj(), /*isCopy=*/nullptr);
memcpy(array_ptr, container.data(), container.size() * sizeof(int8_t));
env->ReleaseByteArrayElements(jarray.obj(), array_ptr, /*mode=*/0);
return jarray;
}
ScopedJavaLocalRef<jintArray> NativeToJavaIntArray(
JNIEnv* env,
rtc::ArrayView<int32_t> container) {
ScopedJavaLocalRef<jintArray> jarray(env, env->NewIntArray(container.size()));
int32_t* array_ptr =
env->GetIntArrayElements(jarray.obj(), /*isCopy=*/nullptr);
memcpy(array_ptr, container.data(), container.size() * sizeof(int32_t));
env->ReleaseIntArrayElements(jarray.obj(), array_ptr, /*mode=*/0);
return jarray;
}
std::vector<int8_t> JavaToNativeByteArray(JNIEnv* env,
const JavaRef<jbyteArray>& jarray) {
int8_t* array_ptr =
env->GetByteArrayElements(jarray.obj(), /*isCopy=*/nullptr);
size_t array_length = env->GetArrayLength(jarray.obj());
std::vector<int8_t> container(array_ptr, array_ptr + array_length);
env->ReleaseByteArrayElements(jarray.obj(), array_ptr, /*mode=*/JNI_ABORT);
return container;
}
std::vector<int32_t> JavaToNativeIntArray(JNIEnv* env,
const JavaRef<jintArray>& jarray) {
int32_t* array_ptr =
env->GetIntArrayElements(jarray.obj(), /*isCopy=*/nullptr);
size_t array_length = env->GetArrayLength(jarray.obj());
std::vector<int32_t> container(array_ptr, array_ptr + array_length);
env->ReleaseIntArrayElements(jarray.obj(), array_ptr, /*mode=*/JNI_ABORT);
return container;
}
ScopedJavaLocalRef<jobjectArray> NativeToJavaBooleanArray(
JNIEnv* env,
const std::vector<bool>& container) {

13
sdk/android/native_api/jni/java_types.h
View File

@ -23,6 +23,7 @@
#include <vector>
#include "absl/types/optional.h"
#include "api/array_view.h"
#include "rtc_base/checks.h"
#include "rtc_base/thread_checker.h"
#include "sdk/android/native_api/jni/scoped_java_ref.h"
@ -220,6 +221,18 @@ ScopedJavaLocalRef<jobjectArray> NativeToJavaObjectArray(
return j_container;
}
ScopedJavaLocalRef<jbyteArray> NativeToJavaByteArray(
JNIEnv* env,
rtc::ArrayView<int8_t> container);
ScopedJavaLocalRef<jintArray> NativeToJavaIntArray(
JNIEnv* env,
rtc::ArrayView<int32_t> container);
std::vector<int8_t> JavaToNativeByteArray(JNIEnv* env,
const JavaRef<jbyteArray>& jarray);
std::vector<int32_t> JavaToNativeIntArray(JNIEnv* env,
const JavaRef<jintArray>& jarray);
ScopedJavaLocalRef<jobjectArray> NativeToJavaBooleanArray(
JNIEnv* env,
const std::vector<bool>& container);

41
sdk/android/native_unittests/java_types_unittest.cc
View File

@ -30,6 +30,47 @@ TEST(JavaTypesTest, TestJavaToNativeStringMap) {
};
EXPECT_EQ(expected, output);
}
TEST(JavaTypesTest, TestNativeToJavaToNativeIntArray) {
JNIEnv* env = AttachCurrentThreadIfNeeded();
std::vector<int32_t> test_data{1, 20, 300};
ScopedJavaLocalRef<jintArray> array = NativeToJavaIntArray(env, test_data);
EXPECT_EQ(test_data, JavaToNativeIntArray(env, array));
}
TEST(JavaTypesTest, TestNativeToJavaToNativeByteArray) {
JNIEnv* env = AttachCurrentThreadIfNeeded();
std::vector<int8_t> test_data{1, 20, 30};
ScopedJavaLocalRef<jbyteArray> array = NativeToJavaByteArray(env, test_data);
EXPECT_EQ(test_data, JavaToNativeByteArray(env, array));
}
TEST(JavaTypesTest, TestNativeToJavaToNativeIntArrayLeakTest) {
JNIEnv* env = AttachCurrentThreadIfNeeded();
std::vector<int32_t> test_data{1, 20, 300};
for (int i = 0; i < 2000; i++) {
ScopedJavaLocalRef<jintArray> array = NativeToJavaIntArray(env, test_data);
EXPECT_EQ(test_data, JavaToNativeIntArray(env, array));
}
}
TEST(JavaTypesTest, TestNativeToJavaToNativeByteArrayLeakTest) {
JNIEnv* env = AttachCurrentThreadIfNeeded();
std::vector<int8_t> test_data{1, 20, 30};
for (int i = 0; i < 2000; i++) {
ScopedJavaLocalRef<jbyteArray> array =
NativeToJavaByteArray(env, test_data);
EXPECT_EQ(test_data, JavaToNativeByteArray(env, array));
}
}
} // namespace
} // namespace test
} // namespace webrtc

14
sdk/android/src/jni/android_network_monitor.cc
View File

@ -99,23 +99,19 @@ static rtc::AdapterType AdapterTypeFromNetworkType(NetworkType network_type) {
static rtc::IPAddress JavaToNativeIpAddress(
JNIEnv* jni,
const JavaRef<jobject>& j_ip_address) {
ScopedJavaLocalRef<jbyteArray> j_addresses =
Java_IPAddress_getAddress(jni, j_ip_address);
size_t address_length = jni->GetArrayLength(j_addresses.obj());
jbyte* addr_array = jni->GetByteArrayElements(j_addresses.obj(), nullptr);
CHECK_EXCEPTION(jni) << "Error during JavaToNativeIpAddress";
std::vector<int8_t> address =
JavaToNativeByteArray(jni, Java_IPAddress_getAddress(jni, j_ip_address));
size_t address_length = address.size();
if (address_length == 4) {
// IP4
struct in_addr ip4_addr;
memcpy(&ip4_addr.s_addr, addr_array, 4);
jni->ReleaseByteArrayElements(j_addresses.obj(), addr_array, JNI_ABORT);
memcpy(&ip4_addr.s_addr, address.data(), 4);
return rtc::IPAddress(ip4_addr);
}
// IP6
RTC_CHECK(address_length == 16);
struct in6_addr ip6_addr;
memcpy(ip6_addr.s6_addr, addr_array, address_length);
jni->ReleaseByteArrayElements(j_addresses.obj(), addr_array, JNI_ABORT);
memcpy(ip6_addr.s6_addr, address.data(), address_length);
return rtc::IPAddress(ip6_addr);
}

7
sdk/android/src/jni/pc/data_channel.cc
View File

@ -146,10 +146,9 @@ static jboolean JNI_DataChannel_Send(JNIEnv* jni,
const JavaParamRef<jobject>& j_dc,
const JavaParamRef<jbyteArray>& data,
jboolean binary) {
jbyte* bytes = jni->GetByteArrayElements(data.obj(), nullptr);
bool ret = ExtractNativeDC(jni, j_dc)->Send(DataBuffer(
rtc::CopyOnWriteBuffer(bytes, jni->GetArrayLength(data.obj())), binary));
jni->ReleaseByteArrayElements(data.obj(), bytes, JNI_ABORT);
std::vector<int8_t> buffer = JavaToNativeByteArray(jni, data);
bool ret = ExtractNativeDC(jni, j_dc)->Send(
DataBuffer(rtc::CopyOnWriteBuffer(buffer.data(), buffer.size()), binary));
return ret;
}

12
sdk/android/src/jni/video_encoder_wrapper.cc
View File

@ -421,17 +421,13 @@ ScopedJavaLocalRef<jobject> VideoEncoderWrapper::ToJavaBitrateAllocation(
jni, jni->NewObjectArray(kMaxSpatialLayers, int_array_class_.obj(),
nullptr /* initial */));
for (int spatial_i = 0; spatial_i < kMaxSpatialLayers; ++spatial_i) {
ScopedJavaLocalRef<jintArray> j_array_spatial_layer(
jni, jni->NewIntArray(kMaxTemporalStreams));
jint* array_spatial_layer = jni->GetIntArrayElements(
j_array_spatial_layer.obj(), nullptr /* isCopy */);
std::array<int32_t, kMaxTemporalStreams> spatial_layer;
for (int temporal_i = 0; temporal_i < kMaxTemporalStreams; ++temporal_i) {
array_spatial_layer[temporal_i] =
allocation.GetBitrate(spatial_i, temporal_i);
spatial_layer[temporal_i] = allocation.GetBitrate(spatial_i, temporal_i);
}
jni->ReleaseIntArrayElements(j_array_spatial_layer.obj(),
array_spatial_layer, JNI_COMMIT);
ScopedJavaLocalRef<jintArray> j_array_spatial_layer =
NativeToJavaIntArray(jni, spatial_layer);
jni->SetObjectArrayElement(j_allocation_array.obj(), spatial_i,
j_array_spatial_layer.obj());
}