Skip to content

Commit 034125f

Browse files
realm-cirealm-ci
authored
Merge 30189ba into master-4.0
2 parents 33e659f + 30189ba commit 034125f

5 files changed

Lines changed: 113 additions & 49 deletions

File tree

‎realm/realm-library/src/main/cpp/io_realm_internal_OsRealmConfig.cpp‎

Lines changed: 38 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@
2626
#include "util.hpp"
2727
#include "jni_util/java_method.hpp"
2828
#include "jni_util/java_class.hpp"
29-
#include "jni_util/java_global_ref.hpp"
29+
#include "jni_util/java_global_weak_ref.hpp"
3030
#include "jni_util/jni_utils.hpp"
3131
#include "jni_util/java_exception_thrower.hpp"
3232

@@ -132,16 +132,26 @@ JNIEXPORT void JNICALL Java_io_realm_internal_OsRealmConfig_nativeSetSchemaConfi
132132
static JavaMethod run_migration_callback_method(
133133
env, get_shared_realm_class(env), "runMigrationCallback",
134134
"(JLio/realm/internal/OsRealmConfig;Lio/realm/internal/SharedRealm$MigrationCallback;J)V", true);
135-
JavaGlobalRef j_config_ref(env, j_config, true);
136-
JavaGlobalRef j_migration_callback_ref(env, j_migration_callback, true);
137-
config.migration_function = [j_config_ref, j_migration_callback_ref](SharedRealm old_realm,
135+
// weak ref to avoid leaks caused by circular refs.
136+
JavaGlobalWeakRef j_config_weak(env, j_config);
137+
JavaGlobalWeakRef j_migration_cb_weak(env, j_migration_callback);
138+
// TODO: It would be great if we can use move constructor in the lambda capture which was introduced in
139+
// c++14. But sadly it seems to be a bug with gcc 4.9 to support it.
140+
config.migration_function = [j_migration_cb_weak, j_config_weak](SharedRealm old_realm,
138141
SharedRealm realm, Schema&) {
139142
JNIEnv* env = JniUtils::get_env(false);
140143
// Java needs a new pointer for the SharedRealm life control.
141144
SharedRealm* new_shared_realm_ptr = new SharedRealm(realm);
142-
env->CallStaticVoidMethod(get_shared_realm_class(env), run_migration_callback_method,
143-
reinterpret_cast<jlong>(new_shared_realm_ptr), j_config_ref.get(),
144-
j_migration_callback_ref.get(), old_realm->schema_version());
145+
JavaGlobalRef config_global = j_config_weak.global_ref(env);
146+
if (!config_global) {
147+
return;
148+
}
149+
150+
j_migration_cb_weak.call_with_local_ref(env, [&](JNIEnv* env, jobject obj) {
151+
env->CallStaticVoidMethod(get_shared_realm_class(env), run_migration_callback_method,
152+
reinterpret_cast<jlong>(new_shared_realm_ptr), config_global.get(), obj,
153+
old_realm->schema_version());
154+
});
145155
TERMINATE_JNI_IF_JAVA_EXCEPTION_OCCURRED(env);
146156
};
147157
}
@@ -162,13 +172,17 @@ JNIEXPORT void JNICALL Java_io_realm_internal_OsRealmConfig_nativeSetCompactOnLa
162172
if (j_compact_on_launch) {
163173
static JavaClass compact_on_launch_class(env, "io/realm/CompactOnLaunchCallback");
164174
static JavaMethod should_compact(env, compact_on_launch_class, "shouldCompact", "(JJ)Z");
165-
JavaGlobalRef java_compact_on_launch_ref(env, j_compact_on_launch);
175+
// weak ref to avoid leaks caused by circular refs.
176+
JavaGlobalWeakRef java_compact_on_launch_weak(env, j_compact_on_launch);
166177

167-
config.should_compact_on_launch_function = [java_compact_on_launch_ref](uint64_t totalBytes,
178+
config.should_compact_on_launch_function = [java_compact_on_launch_weak](uint64_t totalBytes,
168179
uint64_t usedBytes) {
169180
JNIEnv* env = JniUtils::get_env(false);
170-
bool result = env->CallBooleanMethod(java_compact_on_launch_ref.get(), should_compact,
171-
static_cast<jlong>(totalBytes), static_cast<jlong>(usedBytes));
181+
bool result = false;
182+
java_compact_on_launch_weak.call_with_local_ref(env, [&](JNIEnv* env, jobject obj) {
183+
result = env->CallBooleanMethod(obj, should_compact, static_cast<jlong>(totalBytes),
184+
static_cast<jlong>(usedBytes));
185+
});
172186
TERMINATE_JNI_IF_JAVA_EXCEPTION_OCCURRED(env);
173187
return result;
174188
};
@@ -194,15 +208,22 @@ JNIEXPORT void JNICALL Java_io_realm_internal_OsRealmConfig_nativeSetInitializat
194208
static JavaMethod run_initialization_callback_method(
195209
env, get_shared_realm_class(env), "runInitializationCallback",
196210
"(JLio/realm/internal/OsRealmConfig;Lio/realm/internal/SharedRealm$InitializationCallback;)V", true);
197-
JavaGlobalRef j_init_callback_ref(env, j_init_callback, true);
198-
JavaGlobalRef j_config_ref(env, j_config, true);
199-
config.initialization_function = [j_init_callback_ref, j_config_ref](SharedRealm realm) {
211+
// weak ref to avoid leaks caused by circular refs.
212+
JavaGlobalWeakRef j_init_cb_weak(env, j_init_callback);
213+
JavaGlobalWeakRef j_config_weak(env, j_config);
214+
config.initialization_function = [j_init_cb_weak, j_config_weak](SharedRealm realm) {
200215
JNIEnv* env = JniUtils::get_env(false);
201216
// Java needs a new pointer for the SharedRealm life control.
202217
SharedRealm* new_shared_realm_ptr = new SharedRealm(realm);
203-
env->CallStaticVoidMethod(get_shared_realm_class(env), run_initialization_callback_method,
204-
reinterpret_cast<jlong>(new_shared_realm_ptr), j_config_ref.get(),
205-
j_init_callback_ref.get());
218+
JavaGlobalRef config_global_ref = j_config_weak.global_ref(env);
219+
if (!config_global_ref) {
220+
return;
221+
}
222+
j_init_cb_weak.call_with_local_ref(env, [&](JNIEnv* env, jobject obj) {
223+
env->CallStaticVoidMethod(get_shared_realm_class(env), run_initialization_callback_method,
224+
reinterpret_cast<jlong>(new_shared_realm_ptr), config_global_ref.get(),
225+
obj);
226+
});
206227
TERMINATE_JNI_IF_JAVA_EXCEPTION_OCCURRED(env);
207228
};
208229
}

‎realm/realm-library/src/main/cpp/jni_util/java_global_ref.hpp‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ class JavaGlobalRef {
4545
~JavaGlobalRef();
4646

4747
JavaGlobalRef& operator=(JavaGlobalRef&& rhs);
48+
JavaGlobalRef& operator=(JavaGlobalRef& rhs) = delete;
4849
JavaGlobalRef(JavaGlobalRef&);
4950

5051
inline operator bool() const noexcept

‎realm/realm-library/src/main/cpp/jni_util/java_global_weak_ref.cpp‎

Lines changed: 51 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,56 @@
1919

2020
using namespace realm::jni_util;
2121

22-
bool JavaGlobalWeakRef::call_with_local_ref(JNIEnv* env, std::function<Callback> callback)
22+
JavaGlobalWeakRef::JavaGlobalWeakRef()
23+
: m_weak(nullptr)
24+
{
25+
}
26+
27+
JavaGlobalWeakRef::JavaGlobalWeakRef(JNIEnv* env, jobject obj)
28+
: m_weak(obj ? env->NewWeakGlobalRef(obj) : nullptr)
29+
{
30+
}
31+
32+
JavaGlobalWeakRef::~JavaGlobalWeakRef()
33+
{
34+
if (m_weak) {
35+
JniUtils::get_env()->DeleteWeakGlobalRef(m_weak);
36+
}
37+
}
38+
39+
JavaGlobalWeakRef::JavaGlobalWeakRef(JavaGlobalWeakRef&& rhs)
40+
: m_weak(rhs.m_weak)
41+
{
42+
rhs.m_weak = nullptr;
43+
}
44+
45+
JavaGlobalWeakRef& JavaGlobalWeakRef::operator=(JavaGlobalWeakRef&& rhs)
46+
{
47+
this->~JavaGlobalWeakRef();
48+
new (this) JavaGlobalWeakRef(std::move(rhs));
49+
return *this;
50+
}
51+
52+
JavaGlobalWeakRef::JavaGlobalWeakRef(const JavaGlobalWeakRef& rhs)
53+
: m_weak(JniUtils::get_env(true)->NewWeakGlobalRef(rhs.m_weak))
54+
{
55+
}
56+
57+
JavaGlobalWeakRef& JavaGlobalWeakRef::operator=(const JavaGlobalWeakRef& rhs)
58+
{
59+
new (this) JavaGlobalWeakRef(rhs);
60+
return *this;
61+
}
62+
63+
JavaGlobalRef JavaGlobalWeakRef::global_ref(JNIEnv* env) const
64+
{
65+
if (!env) {
66+
env = JniUtils::get_env(true);
67+
}
68+
return JavaGlobalRef(env, m_weak);
69+
}
70+
71+
bool JavaGlobalWeakRef::call_with_local_ref(JNIEnv* env, std::function<Callback> callback) const
2372
{
2473
if (!m_weak) {
2574
return false;
@@ -34,7 +83,7 @@ bool JavaGlobalWeakRef::call_with_local_ref(JNIEnv* env, std::function<Callback>
3483
return true;
3584
}
3685

37-
bool JavaGlobalWeakRef::call_with_local_ref(std::function<Callback> callback)
86+
bool JavaGlobalWeakRef::call_with_local_ref(std::function<Callback> callback) const
3887
{
3988
return call_with_local_ref(JniUtils::get_env(), callback);
4089
}

‎realm/realm-library/src/main/cpp/jni_util/java_global_weak_ref.hpp‎

Lines changed: 11 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -28,48 +28,30 @@ namespace jni_util {
2828
// RAII wrapper for weak global ref.
2929
class JavaGlobalWeakRef {
3030
public:
31-
JavaGlobalWeakRef()
32-
: m_weak(nullptr)
33-
{
34-
}
35-
JavaGlobalWeakRef(JNIEnv* env, jobject obj)
36-
: m_weak(obj ? env->NewWeakGlobalRef(obj) : nullptr)
37-
{
38-
}
39-
~JavaGlobalWeakRef()
40-
{
41-
if (m_weak) {
42-
JniUtils::get_env()->DeleteWeakGlobalRef(m_weak);
43-
}
44-
}
31+
JavaGlobalWeakRef();
32+
JavaGlobalWeakRef(JNIEnv*, jobject);
33+
~JavaGlobalWeakRef();
4534

46-
JavaGlobalWeakRef(JavaGlobalWeakRef&& rhs)
47-
: m_weak(rhs.m_weak)
48-
{
49-
rhs.m_weak = nullptr;
50-
}
51-
JavaGlobalWeakRef& operator=(JavaGlobalWeakRef&& rhs)
52-
{
53-
this->~JavaGlobalWeakRef();
54-
new (this) JavaGlobalWeakRef(std::move(rhs));
55-
return *this;
56-
}
35+
JavaGlobalWeakRef(JavaGlobalWeakRef&&);
36+
JavaGlobalWeakRef& operator=(JavaGlobalWeakRef&&);
5737

58-
JavaGlobalWeakRef(const JavaGlobalWeakRef&) = delete;
59-
JavaGlobalWeakRef& operator=(const JavaGlobalWeakRef&) = delete;
38+
JavaGlobalWeakRef(const JavaGlobalWeakRef&);
39+
JavaGlobalWeakRef& operator=(const JavaGlobalWeakRef&);
6040

6141
inline operator bool() const noexcept
6242
{
6343
return m_weak != nullptr;
6444
}
6545

46+
JavaGlobalRef global_ref(JNIEnv* env = nullptr) const;
47+
6648
using Callback = void(JNIEnv* env, jobject obj);
6749

6850
// Acquire a local ref and run the callback with it if the weak ref is valid. The local ref will be deleted after
6951
// callback finished. Return false if the weak ref is not valid anymore.
70-
bool call_with_local_ref(JNIEnv* env, std::function<Callback> callback);
52+
bool call_with_local_ref(JNIEnv* env, std::function<Callback> callback) const ;
7153
// Try to get an JNIEnv for current thread then run the callback.
72-
bool call_with_local_ref(std::function<Callback> callback);
54+
bool call_with_local_ref(std::function<Callback> callback) const;
7355

7456
private:
7557
jweak m_weak;

‎realm/realm-library/src/main/java/io/realm/internal/OsRealmConfig.java‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,15 @@ OsRealmConfig build() {
146146
// core destructor's thread safety.
147147
private final NativeContext context = new NativeContext();
148148

149+
// Hold a ref to callbacks to make sure they won't be GCed before getting called.
150+
// JNI should only hold a weak ref in the lambda functions.
151+
@SuppressWarnings({"FieldCanBeLocal", "unused"})
152+
private final CompactOnLaunchCallback compactOnLaunchCallback;
153+
@SuppressWarnings({"FieldCanBeLocal", "unused"})
154+
private final SharedRealm.MigrationCallback migrationCallback;
155+
@SuppressWarnings({"FieldCanBeLocal", "unused"})
156+
private final SharedRealm.InitializationCallback initializationCallback;
157+
149158
private OsRealmConfig(final RealmConfiguration config,
150159
boolean autoUpdateNotification,
151160
@Nullable OsSchemaInfo schemaInfo,
@@ -187,15 +196,17 @@ private OsRealmConfig(final RealmConfiguration config,
187196
}
188197
final long schemaVersion = config.getSchemaVersion();
189198
final long nativeSchemaPtr = schemaInfo == null ? 0 : schemaInfo.getNativePtr();
199+
this.migrationCallback = migrationCallback;
190200
nativeSetSchemaConfig(nativePtr, schemaMode.getNativeValue(), schemaVersion, nativeSchemaPtr, migrationCallback);
191201

192202
// Compact on launch
193-
CompactOnLaunchCallback compactOnLaunchCallback = config.getCompactOnLaunchCallback();
203+
this.compactOnLaunchCallback = config.getCompactOnLaunchCallback();
194204
if (compactOnLaunchCallback != null) {
195205
nativeSetCompactOnLaunchCallback(nativePtr, compactOnLaunchCallback);
196206
}
197207

198208
// Initial data transaction
209+
this.initializationCallback = initializationCallback;
199210
if (initializationCallback != null) {
200211
nativeSetInitializationCallback(nativePtr, initializationCallback);
201212
}

0 commit comments

Comments
 (0)