Skip to content

Commit c151300

Browse files
cmelchiorcmelchior
authored
Fix notification exceptions not getting correctly detected in JNI + Fix flaky sync tests (#7559)
1 parent 04ec39d commit c151300

File tree

9 files changed

+150
-108
lines changed

9 files changed

+150
-108
lines changed

‎CHANGELOG.md‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,15 +4,17 @@
44
* None.
55

66
### Fixed
7-
* None.
7+
* [RealmApp] Failing to refresh the access token due to a 401/403 error will now correctly emit an error with `ErrorCode.BAD_AUTHENTICATION` rather than `ErrorCode.PERMISSION_DENIED`. (Realm Core [#4881](https://github.com/realm/realm-core/issues/4881), since 10.6.1)
8+
* [RealmApp] If an object with a null primary key was deleted by another sync client, the exception `KeyNotFound: No such object` could be triggered. ([Realm Core #4885](https://github.com/realm/realm-core/issues/4885), since 10.0.0)
9+
* Exceptions inside change listeners running on background looper threads would crash the Looper with a native `JNI DETECTED ERROR IN APPLICATION: JNI NewLocalRef called with pending exception` instead of the original Java exception. This could also happen when canceling a corutine using a background looper as a Dispatcher.
810

911
### Compatibility
1012
* File format: Generates Realms with format v22. Unsynced Realms will be upgraded from Realm Java 2.0 and later. Synced Realms can only be read and upgraded if created with Realm Java v10.0.0-BETA.1.
1113
* APIs are backwards compatible with all previous release of realm-java in the 10.6.y series.
1214
* Realm Studio 11.0.0-alpha.0 or above is required to open Realms created by this version.
1315

1416
### Internal
15-
* None.
17+
* Updated to Realm Core 11.4.0, commit: 9b2f67c24581503486d8e1d2066fd7e8c5fd1491.
1618

1719

1820
## 10.8.0 (2021-08-27)

‎realm/realm-library/src/androidTest/java/io/realm/internal/RealmNotifierTests.java‎

Lines changed: 105 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,11 @@
1616
package io.realm.internal;
1717

1818

19+
import static junit.framework.Assert.assertEquals;
20+
import static junit.framework.Assert.assertFalse;
21+
import static junit.framework.Assert.fail;
22+
import static org.junit.Assert.assertTrue;
23+
1924
import androidx.test.ext.junit.runners.AndroidJUnit4;
2025

2126
import org.junit.After;
@@ -24,28 +29,26 @@
2429
import org.junit.Test;
2530
import org.junit.runner.RunWith;
2631

32+
import java.util.UUID;
2733
import java.util.concurrent.atomic.AtomicBoolean;
2834
import java.util.concurrent.atomic.AtomicInteger;
2935

3036
import javax.annotation.Nullable;
3137

3238
import io.realm.RealmChangeListener;
3339
import io.realm.RealmConfiguration;
34-
import io.realm.internal.android.AndroidRealmNotifier;
35-
import io.realm.rule.RunInLooperThread;
36-
import io.realm.rule.RunTestInLooperThread;
3740
import io.realm.TestRealmConfigurationFactory;
38-
39-
import static junit.framework.Assert.assertEquals;
40-
import static junit.framework.Assert.assertFalse;
41-
import static junit.framework.Assert.fail;
41+
import io.realm.internal.android.AndroidRealmNotifier;
42+
import io.realm.rule.BlockingLooperThread;
4243

4344
@RunWith(AndroidJUnit4.class)
4445
public class RealmNotifierTests {
4546
@Rule
4647
public final TestRealmConfigurationFactory configFactory = new TestRealmConfigurationFactory();
47-
@Rule
48-
public final RunInLooperThread looperThread = new RunInLooperThread();
48+
49+
private final BlockingLooperThread looperThread = new BlockingLooperThread();
50+
51+
private RealmConfiguration realmConfig;
4952

5053
private Capabilities capabilitiesCanDeliver = new Capabilities() {
5154
@Override
@@ -65,6 +68,7 @@ public boolean isMainThread() {
6568

6669
@Before
6770
public void setUp() throws Exception {
71+
realmConfig = configFactory.createConfiguration(UUID.randomUUID().toString());
6872
}
6973

7074
@After
@@ -78,35 +82,37 @@ private OsSharedRealm getSharedRealm(RealmConfiguration config) {
7882
}
7983

8084
@Test
81-
@RunTestInLooperThread
8285
public void post() {
83-
RealmNotifier notifier = new AndroidRealmNotifier(null, capabilitiesCanDeliver);
84-
notifier.post(new Runnable() {
85-
@Override
86-
public void run() {
87-
looperThread.testComplete();
88-
}
86+
looperThread.runBlocking(() -> {
87+
RealmNotifier notifier = new AndroidRealmNotifier(null, capabilitiesCanDeliver);
88+
notifier.post(new Runnable() {
89+
@Override
90+
public void run() {
91+
looperThread.testComplete();
92+
}
93+
});
8994
});
9095
}
9196

9297
// Callback is immediately called when commitTransaction for local changes.
9398
@Test
94-
@RunTestInLooperThread
9599
public void addChangeListener_byLocalChanges() {
96-
final AtomicBoolean commitReturns = new AtomicBoolean(false);
97-
OsSharedRealm sharedRealm = getSharedRealm(looperThread.getConfiguration());
98-
sharedRealm.realmNotifier.addChangeListener(sharedRealm, new RealmChangeListener<OsSharedRealm>() {
99-
@Override
100-
public void onChange(OsSharedRealm sharedRealm) {
101-
// Transaction has been committed in core, but commitTransaction hasn't returned in java.
102-
assertFalse(commitReturns.get());
103-
sharedRealm.close();
104-
looperThread.testComplete();
105-
}
100+
looperThread.runBlocking(() -> {
101+
final AtomicBoolean commitReturns = new AtomicBoolean(false);
102+
OsSharedRealm sharedRealm = getSharedRealm(realmConfig);
103+
sharedRealm.realmNotifier.addChangeListener(sharedRealm, new RealmChangeListener<OsSharedRealm>() {
104+
@Override
105+
public void onChange(OsSharedRealm sharedRealm) {
106+
// Transaction has been committed in core, but commitTransaction hasn't returned in java.
107+
assertFalse(commitReturns.get());
108+
sharedRealm.close();
109+
looperThread.testComplete();
110+
}
111+
});
112+
sharedRealm.beginTransaction();
113+
sharedRealm.commitTransaction();
114+
commitReturns.set(true);
106115
});
107-
sharedRealm.beginTransaction();
108-
sharedRealm.commitTransaction();
109-
commitReturns.set(true);
110116
}
111117

112118
private void makeRemoteChanges(final RealmConfiguration config) {
@@ -118,61 +124,83 @@ private void makeRemoteChanges(final RealmConfiguration config) {
118124
}
119125

120126
@Test
121-
@RunTestInLooperThread
122127
public void addChangeListener_byRemoteChanges() {
123-
// To catch https://github.com/realm/realm-java/pull/4037 CI failure.
124-
// In this case, object store should not send more than 100 notifications.
125-
final int TIMES = 100;
126-
final AtomicInteger commitCounter = new AtomicInteger(0);
127-
final AtomicInteger listenerCounter = new AtomicInteger(0);
128-
129-
looperThread.getRealm().close();
130-
131-
OsSharedRealm sharedRealm = getSharedRealm(looperThread.getConfiguration());
132-
looperThread.keepStrongReference(sharedRealm);
133-
sharedRealm.realmNotifier.addChangeListener(sharedRealm, new RealmChangeListener<OsSharedRealm>() {
134-
@Override
135-
public void onChange(OsSharedRealm sharedRealm) {
136-
int commits = commitCounter.get();
137-
int listenerCount = listenerCounter.addAndGet(1);
138-
assertEquals(commits, listenerCount);
139-
if (commits == TIMES) {
140-
sharedRealm.close();
141-
looperThread.testComplete();
142-
} else {
143-
makeRemoteChanges(looperThread.getConfiguration());
144-
commitCounter.getAndIncrement();
128+
looperThread.runBlocking(() -> {
129+
// To catch https://github.com/realm/realm-java/pull/4037 CI failure.
130+
// In this case, object store should not send more than 100 notifications.
131+
final int TIMES = 100;
132+
final AtomicInteger commitCounter = new AtomicInteger(0);
133+
final AtomicInteger listenerCounter = new AtomicInteger(0);
134+
135+
OsSharedRealm sharedRealm = getSharedRealm(realmConfig);
136+
looperThread.keepStrongReference(sharedRealm);
137+
sharedRealm.realmNotifier.addChangeListener(sharedRealm, new RealmChangeListener<OsSharedRealm>() {
138+
@Override
139+
public void onChange(OsSharedRealm sharedRealm) {
140+
int commits = commitCounter.get();
141+
int listenerCount = listenerCounter.addAndGet(1);
142+
assertEquals(commits, listenerCount);
143+
if (commits == TIMES) {
144+
sharedRealm.close();
145+
looperThread.testComplete();
146+
} else {
147+
makeRemoteChanges(realmConfig);
148+
commitCounter.getAndIncrement();
149+
}
145150
}
146-
}
151+
});
152+
makeRemoteChanges(realmConfig);
153+
commitCounter.getAndIncrement();
147154
});
148-
makeRemoteChanges(looperThread.getConfiguration());
149-
commitCounter.getAndIncrement();
155+
}
156+
157+
// Ensure that exceptions in changelisteners do not cause native crashes, but instead
158+
// propagate correctly to end users
159+
@Test
160+
public void addChangeListener_exceptionsPropagateCorrectly() {
161+
try {
162+
looperThread.runBlocking(() -> {
163+
OsSharedRealm sharedRealm = getSharedRealm(realmConfig);
164+
looperThread.closeAfterTest(sharedRealm);
165+
sharedRealm.realmNotifier.addChangeListener(sharedRealm, new RealmChangeListener<OsSharedRealm>() {
166+
@Override
167+
public void onChange(OsSharedRealm sharedRealm) {
168+
throw new RuntimeException("BOOM");
169+
}
170+
});
171+
makeRemoteChanges(realmConfig);
172+
});
173+
fail();
174+
} catch (RuntimeException ex) {
175+
assertTrue(ex.getMessage().contains("BOOM"));
176+
}
150177
}
151178

152179
@Test
153-
@RunTestInLooperThread
154180
public void removeChangeListeners() {
155-
OsSharedRealm sharedRealm = getSharedRealm(looperThread.getConfiguration());
156-
Integer dummyObserver = 1;
157-
looperThread.keepStrongReference(dummyObserver);
158-
looperThread.keepStrongReference(sharedRealm);
159-
sharedRealm.realmNotifier.addChangeListener(dummyObserver, new RealmChangeListener<Integer>() {
160-
@Override
161-
public void onChange(Integer dummy) {
162-
fail();
163-
}
164-
});
165-
sharedRealm.realmNotifier.addChangeListener(sharedRealm, new RealmChangeListener<OsSharedRealm>() {
166-
@Override
167-
public void onChange(OsSharedRealm sharedRealm) {
168-
sharedRealm.close();
169-
looperThread.testComplete();
170-
}
171-
});
181+
looperThread.runBlocking(() -> {
182+
OsSharedRealm sharedRealm = getSharedRealm(realmConfig);
183+
Integer dummyObserver = 1;
184+
looperThread.keepStrongReference(dummyObserver);
185+
looperThread.keepStrongReference(sharedRealm);
186+
sharedRealm.realmNotifier.addChangeListener(dummyObserver, new RealmChangeListener<Integer>() {
187+
@Override
188+
public void onChange(Integer dummy) {
189+
fail();
190+
}
191+
});
192+
sharedRealm.realmNotifier.addChangeListener(sharedRealm, new RealmChangeListener<OsSharedRealm>() {
193+
@Override
194+
public void onChange(OsSharedRealm sharedRealm) {
195+
sharedRealm.close();
196+
looperThread.testComplete();
197+
}
198+
});
172199

173-
// This should only remove the listeners related with dummyObserver
174-
sharedRealm.realmNotifier.removeChangeListeners(dummyObserver);
200+
// This should only remove the listeners related with dummyObserver
201+
sharedRealm.realmNotifier.removeChangeListeners(dummyObserver);
175202

176-
makeRemoteChanges(looperThread.getConfiguration());
203+
makeRemoteChanges(realmConfig);
204+
});
177205
}
178206
}

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

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -310,7 +310,7 @@ JNIEXPORT jstring JNICALL Java_io_realm_internal_OsRealmConfig_nativeCreateAndSe
310310
jstring jerror_category = to_jstring(env, error_category);
311311
jstring jerror_message = to_jstring(env, error_message);
312312
jstring jclient_reset_path_info = to_jstring(env, client_reset_path_info);
313-
jstring jsession_path = to_jstring(env, session.get()->path());
313+
jstring jsession_path = to_jstring(env, session->path());
314314
env->CallVoidMethod(sync_service_object.get(),
315315
java_error_callback_method,
316316
jerror_category,
@@ -373,11 +373,6 @@ JNIEXPORT jstring JNICALL Java_io_realm_internal_OsRealmConfig_nativeCreateAndSe
373373
}
374374
}
375375

376-
if (!config.encryption_key.empty()) {
377-
config.sync_config->realm_encryption_key = std::array<char, 64>();
378-
std::copy_n(config.encryption_key.begin(), 64, config.sync_config->realm_encryption_key->begin());
379-
}
380-
381376
// return to_jstring(env, config.sync_config->realm_url.c_str());
382377
// FIXME: We must return the realm url here for proxy support to work
383378
return to_jstring(env, "");

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

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ JNIEXPORT void JNICALL Java_io_realm_mongodb_sync_Sync_nativeReset(JNIEnv* env,
3737
{
3838
try {
3939
auto app = *reinterpret_cast<std::shared_ptr<app::App>*>(j_app_ptr);
40+
app->sync_manager()->wait_for_sessions_to_terminate();
4041
app->sync_manager()->reset_for_testing();
4142
app::App::clear_cached_apps();
4243
}
@@ -72,15 +73,6 @@ JNIEXPORT void JNICALL Java_io_realm_mongodb_sync_Sync_nativeReconnect(JNIEnv* e
7273
CATCH_STD()
7374
}
7475

75-
JNIEXPORT void JNICALL Java_io_realm_mongodb_sync_Sync_nativeCreateSession(JNIEnv* env, jclass, jlong j_native_config_ptr)
76-
{
77-
try {
78-
auto& config = *reinterpret_cast<Realm::Config*>(j_native_config_ptr);
79-
_impl::RealmCoordinator::get_coordinator(config)->create_session(config);
80-
}
81-
CATCH_STD()
82-
}
83-
8476
JNIEXPORT jstring JNICALL Java_io_realm_mongodb_sync_Sync_nativeGetPathForRealm(JNIEnv* env,
8577
jclass,
8678
jlong j_app_ptr,

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

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,7 @@ void JavaBindingContext::did_change(std::vector<BindingContext::ObserverState> c
4343
bool version_changed)
4444
{
4545
auto env = JniUtils::get_env();
46-
47-
if (JniUtils::get_env()->ExceptionCheck()) {
46+
if (env->ExceptionCheck()) {
4847
return;
4948
}
5049
if (version_changed) {
@@ -62,6 +61,9 @@ void JavaBindingContext::schema_did_change(Schema const&)
6261
return;
6362
}
6463
auto env = JniUtils::get_env(false);
64+
if (env->ExceptionCheck()) {
65+
return;
66+
}
6567
static JavaMethod on_schema_changed_method(env, JavaClassGlobalDef::shared_realm_schema_change_callback(),
6668
"onSchemaChanged", "()V");
6769
m_schema_changed_callback.call_with_local_ref(
@@ -75,6 +77,9 @@ void JavaBindingContext::set_schema_changed_callback(JNIEnv* env, jobject schema
7577

7678
void JavaBindingContext::will_send_notifications() {
7779
auto env = JniUtils::get_env();
80+
if (env->ExceptionCheck()) {
81+
return;
82+
}
7883
m_java_notifier.call_with_local_ref(env, [&](JNIEnv*, jobject notifier_obj) {
7984
static JavaMethod realm_notifier_will_send_notifications(env, JavaClassGlobalDef::realm_notifier(),
8085
"willSendNotifications", "()V");
@@ -84,6 +89,9 @@ void JavaBindingContext::will_send_notifications() {
8489

8590
void JavaBindingContext::did_send_notifications() {
8691
auto env = JniUtils::get_env();
92+
if (env->ExceptionCheck()) {
93+
return;
94+
}
8795
m_java_notifier.call_with_local_ref(env, [&](JNIEnv*, jobject notifier_obj) {
8896
static JavaMethod realm_notifier_did_send_notifications(env, JavaClassGlobalDef::realm_notifier(),
8997
"didSendNotifications", "()V");

‎realm/realm-library/src/main/cpp/realm-core‎

Submodule realm-core updated 82 files

‎realm/realm-library/src/objectServer/java/io/realm/mongodb/sync/Sync.java‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -175,12 +175,6 @@ public synchronized SyncSession getOrCreateSession(SyncConfiguration syncConfigu
175175
RealmLog.debug("First session created. Adding network listener.");
176176
NetworkStateReceiver.addListener(networkListener);
177177
}
178-
// The underlying session will be created as part of opening the Realm, but this approach
179-
// does not work when using `Realm.getInstanceAsync()` in combination with AsyncOpen.
180-
//
181-
// So instead we manually create the underlying native session.
182-
OsRealmConfig config = new OsRealmConfig.Builder(syncConfiguration).build();
183-
nativeCreateSession(config.getNativePtr());
184178
}
185179

186180
return session;
@@ -314,6 +308,5 @@ void simulateClientReset(SyncSession session) {
314308
private static native void nativeReset(long appNativePointer);
315309
private static native void nativeSimulateSyncError(long appNativePointer, String realmPath, int errorCode, String errorMessage, boolean isFatal);
316310
private static native void nativeReconnect(long appNativePointer);
317-
private static native void nativeCreateSession(long nativeConfigPtr);
318311
private static native String nativeGetPathForRealm(long appNativePointer, String userId, String partitionValue, @Nullable String overrideFileName);
319312
}

‎realm/realm-library/src/objectServer/java/io/realm/mongodb/sync/SyncSession.java‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -428,6 +428,17 @@ public synchronized void removeConnectionChangeListener(ConnectionListener liste
428428
}
429429

430430
synchronized void close() {
431+
// Clear any running listeners as they might prevent the underlying session
432+
// from correctly closing.
433+
if (!connectionListeners.isEmpty()) {
434+
connectionListeners.clear();
435+
nativeRemoveConnectionListener(appNativePointer, nativeConnectionListenerToken, configuration.getPath());
436+
}
437+
for (Long token : progressListenerToOsTokenMap.values()) {
438+
nativeRemoveProgressListener(appNativePointer, configuration.getPath(), token);
439+
}
440+
listenerIdToProgressListenerMap.clear();
441+
progressListenerToOsTokenMap.clear();
431442
isClosed = true;
432443
}
433444

0 commit comments

Comments
 (0)