Skip to content

Commit

Permalink
Fix JVM memory leaks (#1526)
Browse files Browse the repository at this point in the history
  • Loading branch information
@clementetb
clementetb authored Oct 6, 2023
1 parent b3406e3 commit 330a06c
Show file tree
Hide file tree
Showing 6 changed files with 137 additions and 57 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

### Fixed
* `Realm.getNumberOfActiveVersions` now returns the actual number of active versions. (Core issue [#6960](https://github.com/realm/realm-core/pull/6960))
* Fix memory leaks on the JVM platform, see PR for more information. (Issue [#1526](https://github.com/realm/realm-kotlin/pull/1526))

### Compatibility
* File format: Generates Realms with file format v23.
Expand Down
12 changes: 6 additions & 6 deletions packages/cinterop/src/commonMain/kotlin/io/realm/kotlin/internal/interop/MemAllocator.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,12 +93,6 @@ interface MemAllocator {
* Instantiates a [RealmValue] representing a `realm_value_t` of type `RLM_TYPE_LINK`.
*/
fun realmObjectTransport(value: RealmObjectInterop?): RealmValue

/**
* Instantiates a [RealmQueryArgumentList] representing a `realm_query_arg_t` that describe and
* references the incoming [RealmValueList] arguments.
*/
fun queryArgsOf(queryArgs: List<RealmQueryArgument>): RealmQueryArgumentList
}

/**
Expand All @@ -118,6 +112,12 @@ interface MemTrackingAllocator : MemAllocator {
*/
fun byteArrayTransport(value: ByteArray?): RealmValue

/**
* Instantiates a [RealmQueryArgumentList] representing a `realm_query_arg_t` that describe and
* references the incoming [RealmValueList] arguments.
*/
fun queryArgsOf(queryArgs: List<RealmQueryArgument>): RealmQueryArgumentList

/**
* Frees resources linked to this allocator. See implementations for more details.
*/
Expand Down
85 changes: 64 additions & 21 deletions packages/cinterop/src/jvm/kotlin/io/realm/kotlin/internal/interop/RealmInterop.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package io.realm.kotlin.internal.interop

import io.realm.kotlin.internal.interop.Constants.ENCRYPTION_KEY_LENGTH
import io.realm.kotlin.internal.interop.RealmInterop.cptr
import io.realm.kotlin.internal.interop.sync.ApiKeyWrapper
import io.realm.kotlin.internal.interop.sync.AuthProvider
import io.realm.kotlin.internal.interop.sync.CoreConnectionState
Expand Down Expand Up @@ -117,7 +118,35 @@ actual object RealmInterop {
realmc.classArray_setitem(cclasses, i, cclass)
realmc.propertyArrayArray_setitem(cproperties, i, classProperties)
}
return LongPointerWrapper(realmc.realm_schema_new(cclasses, count.toLong(), cproperties))
try {
return LongPointerWrapper(realmc.realm_schema_new(cclasses, count.toLong(), cproperties))
} finally {
// Clean up classes
for (classIndex in 0 until count) {
val classInfo = realmc.classArray_getitem(cclasses, classIndex)

// Clean properties
val propertyArray = realmc.propertyArrayArray_getitem(cproperties, classIndex)

val propertyCount = classInfo.getNum_properties() + classInfo.getNum_computed_properties()
for (propertyIndex in 0 until propertyCount) {
val property = realmc.propertyArray_getitem(propertyArray, propertyIndex.toInt())

realmc.realm_property_info_t_cleanup(property)
property.delete()
}

realmc.delete_propertyArray(propertyArray)

// end clean properties

realmc.realm_class_info_t_cleanup(classInfo)
classInfo.delete()
}

realmc.delete_propertyArrayArray(cproperties)
realmc.delete_classArray(cclasses)
}
}

actual fun realm_config_new(): RealmConfigurationPointer {
Expand Down Expand Up @@ -316,23 +345,27 @@ actual object RealmInterop {
val properties = realmc.new_propertyArray(max.toInt())
val outCount = longArrayOf(0)
realmc.realm_get_class_properties(realm.cptr(), classKey.key, properties, max, outCount)
return if (outCount[0] > 0) {
(0 until outCount[0]).map { i ->
with(realmc.propertyArray_getitem(properties, i.toInt())) {
PropertyInfo(
name,
public_name,
PropertyType.from(type),
CollectionType.from(collection_type),
link_target,
link_origin_property_name,
PropertyKey(key),
flags
)
try {
return if (outCount[0] > 0) {
(0 until outCount[0]).map { i ->
with(realmc.propertyArray_getitem(properties, i.toInt())) {
PropertyInfo(
name,
public_name,
PropertyType.from(type),
CollectionType.from(collection_type),
link_target,
link_origin_property_name,
PropertyKey(key),
flags
)
}
}
} else {
emptyList()
}
} else {
emptyList()
} finally {
realmc.delete_propertyArray(properties)
}
}

Expand Down Expand Up @@ -922,6 +955,8 @@ actual object RealmInterop {
builder.initIndicesArray(builder::modificationIndices, modificationIndices)
builder.initIndicesArray(builder::modificationIndicesAfter, modificationIndicesAfter)
builder.movesCount = movesCount[0].toInt()

realmc.delete_collectionMoveArray(moves)
}

actual fun <T, R> realm_collection_changes_get_ranges(
Expand Down Expand Up @@ -969,6 +1004,12 @@ actual object RealmInterop {
builder.initRangesArray(builder::insertionRanges, insertionRanges, insertRangesCount[0])
builder.initRangesArray(builder::modificationRanges, modificationRanges, modificationRangesCount[0])
builder.initRangesArray(builder::modificationRangesAfter, modificationRangesAfter, modificationRangesCount[0])

realmc.delete_indexRangeArray(insertionRanges)
realmc.delete_indexRangeArray(modificationRanges)
realmc.delete_indexRangeArray(modificationRangesAfter)
realmc.delete_indexRangeArray(deletionRanges)
realmc.delete_collectionMoveArray(moves)
}

actual fun <R> realm_dictionary_get_changes(
Expand Down Expand Up @@ -1107,6 +1148,8 @@ actual object RealmInterop {
}
} else {
emptyList()
}.also {
realmc.delete_identityArray(keys)
}
}

Expand Down Expand Up @@ -1609,7 +1652,7 @@ actual object RealmInterop {
realm: RealmPointer,
classKey: ClassKey,
query: String,
args: RealmQueryArgumentList
args: RealmQueryArgumentList,
): RealmQueryPointer {
return LongPointerWrapper(
realmc.realm_query_parse(
Expand All @@ -1625,7 +1668,7 @@ actual object RealmInterop {
actual fun realm_query_parse_for_results(
results: RealmResultsPointer,
query: String,
args: RealmQueryArgumentList
args: RealmQueryArgumentList,
): RealmQueryPointer {
return LongPointerWrapper(
realmc.realm_query_parse_for_results(
Expand All @@ -1640,7 +1683,7 @@ actual object RealmInterop {
actual fun realm_query_parse_for_list(
list: RealmListPointer,
query: String,
args: RealmQueryArgumentList
args: RealmQueryArgumentList,
): RealmQueryPointer {
return LongPointerWrapper(
realmc.realm_query_parse_for_list(
Expand All @@ -1655,7 +1698,7 @@ actual object RealmInterop {
actual fun realm_query_parse_for_set(
set: RealmSetPointer,
query: String,
args: RealmQueryArgumentList
args: RealmQueryArgumentList,
): RealmQueryPointer {
return LongPointerWrapper(
realmc.realm_query_parse_for_set(
Expand Down Expand Up @@ -1693,7 +1736,7 @@ actual object RealmInterop {
actual fun realm_query_append_query(
query: RealmQueryPointer,
filter: String,
args: RealmQueryArgumentList
args: RealmQueryArgumentList,
): RealmQueryPointer {
return LongPointerWrapper(
realmc.realm_query_append_query(query.cptr(), filter, args.size, args.head)
Expand Down
66 changes: 41 additions & 25 deletions packages/cinterop/src/jvm/kotlin/io/realm/kotlin/internal/interop/RealmValueAllocator.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,28 +93,6 @@ object JvmMemAllocator : MemAllocator {
link = realmc.realm_object_as_link(it.objectPointer.cptr())
}

override fun queryArgsOf(queryArgs: List<RealmQueryArgument>): RealmQueryArgumentList {
val cArgs = realmc.new_queryArgArray(queryArgs.size)
queryArgs.mapIndexed { index, arg ->
val queryArg = realm_query_arg_t().apply {
when (arg) {
is RealmQueryListArgument -> {
nb_args = arg.arguments.size.toLong()
is_list = true
this.arg = arg.arguments.head
}
is RealmQuerySingleArgument -> {
nb_args = 1
is_list = false
this.arg = arg.argument.value
}
}
}
realmc.queryArgArray_setitem(cArgs, index, queryArg)
}
return RealmQueryArgumentList(queryArgs.size.toLong(), cArgs)
}

private inline fun <T> createTransport(
value: T?,
type: Int,
Expand Down Expand Up @@ -151,6 +129,32 @@ class JvmMemTrackingAllocator : MemAllocator by JvmMemAllocator, MemTrackingAllo
}
}

override fun queryArgsOf(queryArgs: List<RealmQueryArgument>): RealmQueryArgumentList {
val cArgs = realmc.new_queryArgArray(queryArgs.size)
queryArgs.mapIndexed { index, arg ->
val queryArg = realm_query_arg_t().apply {
when (arg) {
is RealmQueryListArgument -> {
nb_args = arg.arguments.size.toLong()
is_list = true
this.arg = arg.arguments.head

scope.manageQueryListArgument(arg)
}
is RealmQuerySingleArgument -> {
nb_args = 1
is_list = false
this.arg = arg.argument.value
}
}
}
realmc.queryArgArray_setitem(cArgs, index, queryArg)
}
return RealmQueryArgumentList(queryArgs.size.toLong(), cArgs).also {
scope.manageQueryArgumentList(it)
}
}

/**
* Frees resources linked to this allocator's [scope], more specifically strings and binary
* buffers. See [MemScope.free] for more details.
Expand Down Expand Up @@ -180,16 +184,28 @@ class JvmMemTrackingAllocator : MemAllocator by JvmMemAllocator, MemTrackingAllo
* copied out afterwards).
*/
class MemScope {
val values: MutableSet<RealmValueT> = mutableSetOf()
val values: MutableSet<Any> = mutableSetOf()

fun manageRealmValue(value: RealmValueT): RealmValueT {
values.add(value)
return value
}

fun manageQueryArgumentList(value: RealmQueryArgumentList): RealmQueryArgumentList = value.also {
values.add(value)
}

fun manageQueryListArgument(value: RealmQueryListArgument): RealmQueryListArgument = value.also {
values.add(value)
}

fun free() {
values.map {
realmc.realm_value_t_cleanup(it)
values.forEach {
when (it) {
is RealmValueT -> realmc.realm_value_t_cleanup(it)
is RealmQueryArgumentList -> realmc.delete_queryArgArray(it.head)
is RealmQueryListArgument -> realmc.delete_valueArray(it.arguments.head)
}
}
}
}
Expand Down
24 changes: 19 additions & 5 deletions packages/jni-swig-stub/src/main/jni/realm_api_helpers.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -721,10 +721,10 @@ jobject convert_to_jvm_sync_error(JNIEnv* jenv, const realm_sync_error_t& error)
jboolean is_unrecognized_by_client = error.is_unrecognized_by_client;
jboolean is_client_reset_requested = error.is_client_reset_requested;

auto user_info_map = new std::map<std::string, std::string>();
auto user_info_map = std::map<std::string, std::string>();
for (int i = 0; i < error.user_info_length; i++) {
realm_sync_error_user_info_t user_info = error.user_info_map[i];
user_info_map->insert(std::make_pair(user_info.key, user_info.value));
user_info_map.insert(std::make_pair(user_info.key, user_info.value));
}

static JavaMethod core_compensating_write_info_constructor(
Expand Down Expand Up @@ -766,16 +766,16 @@ jobject convert_to_jvm_sync_error(JNIEnv* jenv, const realm_sync_error_t& error)
// mark the file for deletion. Having 'original path' in the user_info_map is a side effect of
// using the same code for client reset.
if (error.user_info_length > 0) {
auto end_it = user_info_map->end();
auto end_it = user_info_map.end();

auto original_it = user_info_map->find(error.c_original_file_path_key);
auto original_it = user_info_map.find(error.c_original_file_path_key);
if (end_it != original_it) {
auto original_file_path = original_it->second;
joriginal_file_path = to_jstring(jenv, original_file_path);
}

// Sync errors may not have the path to the recovery file unless a Client Reset is requested
auto recovery_it = user_info_map->find(error.c_recovery_file_path_key);
auto recovery_it = user_info_map.find(error.c_recovery_file_path_key);
if (error.is_client_reset_requested && (end_it != recovery_it)) {
auto recovery_file_path = recovery_it->second;
jrecovery_file_path = to_jstring(jenv, recovery_file_path);
Expand Down Expand Up @@ -1053,3 +1053,17 @@ realm_scheduler_t*
realm_create_generic_scheduler() {
return new realm_scheduler_t { realm::util::Scheduler::make_dummy() };
}

void
realm_property_info_t_cleanup(realm_property_info_t* value) {
delete[] value->link_origin_property_name;
delete[] value->link_target;
delete[] value->name;
delete[] value->public_name;
}

void
realm_class_info_t_cleanup(realm_class_info_t * value) {
delete[] value->primary_key;
delete[] value->name;
}
6 changes: 6 additions & 0 deletions packages/jni-swig-stub/src/main/jni/realm_api_helpers.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -137,4 +137,10 @@ realm_sync_thread_error(realm_userdata_t userdata, const char* error);
realm_scheduler_t*
realm_create_generic_scheduler();

void
realm_property_info_t_cleanup(realm_property_info_t* value);

void
realm_class_info_t_cleanup(realm_class_info_t * value);

#endif //TEST_REALM_API_HELPERS_H

0 comments on commit 330a06c

Please sign in to comment.