realm / realm-java

Realm is a mobile database: a replacement for SQLite & ORMs
http://realm.io
Apache License 2.0
11.47k stars 1.75k forks source link

Odd change in RealmChangeListener behavior and version consistency on background looper threads #4878

Closed Zhuinden closed 7 years ago

Zhuinden commented 7 years ago

Goal

What do you want to achieve?

Be notified of changes on background looper threads, and be notified if Results has changed from writes.

Expected Results

If write happens, but Realm version is not yet updated (is an older version of Realm), then initially get the results from older version, then receive change notification when Realm version is updated by the write.

Actual Results

Somewhere after Realm 3.x (used to work with 2.3.0, did not yet work in 1.2.0), apparently it broke in the sense that the Results is created, but when Realm version upgrades, it does not detect that this results was an older version, so change listener is not called.

Steps & Code to Reproduce

In my simple-stack-example-mvp, I had to add realm.refresh() to force RealmResults to be up to date after obtaining them, even though RealmChangeListener is added to it later.

So pending write did not call RealmChangeListener when Realm was updated.

    public Observable<List<Task>> getTasks() {
        return Observable.create((ObservableOnSubscribe<List<Task>>) emitter -> {
            Realm realm = Realm.getDefaultInstance();
            realm.refresh(); // <-- I had to add this to force Realm to return new results, but previously I received change notification when Results was updated!
            final RealmResults<DbTask> dbTasks = realm.where(DbTask.class).findAllSorted(DbTaskFields.ID, Sort.ASCENDING);
            final RealmChangeListener<RealmResults<DbTask>> realmChangeListener = element -> {
                if(!emitter.isDisposed()) {
                    List<Task> tasks = mapFrom(element);
                    if(!emitter.isDisposed()) {
                        emitter.onNext(tasks);
                    }
                }
            };
            emitter.setDisposable(Disposables.fromAction(() -> {
                if(dbTasks.isValid()) {
                    dbTasks.removeChangeListener(realmChangeListener);
                }
                realm.close();
            }));
            dbTasks.addChangeListener(realmChangeListener);
            emitter.onNext(mapFrom(dbTasks));
        }).subscribeOn(looperScheduler.getScheduler()).unsubscribeOn(looperScheduler.getScheduler());
    }

Version of Realm and tooling

Realm version(s): 3.1.3 and 3.4.0

Realm sync feature enabled: no

Android Studio version: irrelevant

Which Android version and device: Nexus 5X, 7.1.1

kneth commented 7 years ago

@Zhuinden @beeender will have a closer look early next week.

cmelchior commented 7 years ago

Hey @Zhuinden

Can you try adding

RealmLog.error("Count: " + Realm.getLocalInstanceCount(Realm.getDefaultConfiguration()));

Just before you call Realm.getDefaultInstance(). My guess is tht you are getting a cached instance that hasn't been updated yet.

Zhuinden commented 7 years ago

I do have an open Realm instance on the same thread to avoid having to recreate the cache on the looper thread each time, but it's a looper thread, so there shouldn't be race condition involved as long as results change listener is called if results was updated.

I'll get to checking what gets called when when this weekend.

Zhuinden commented 7 years ago

...hmm on this computer I seem to be unlucky with reproducing -_-

07-01 16:38:14.104 3261-3273/com.zhuinden.simplestackdemoexamplemvp I/DatabaseManager: Opened global Realm for looper thread at [2094113317959], instance count is [1]
07-01 16:38:14.128 3261-3273/com.zhuinden.simplestackdemoexamplemvp I/TaskRepository: Opened Realm for tasks with instance count [2] at [2094135316004]
07-01 16:38:14.128 3261-3273/com.zhuinden.simplestackdemoexamplemvp I/TaskRepository: Emit initial tasks [2094136154275]
07-01 16:38:43.128 3261-3273/com.zhuinden.simplestackdemoexamplemvp I/TaskRepository: Destroying Tasks Results at [2123134837595]
07-01 16:38:49.636 3261-3297/com.zhuinden.simplestackdemoexamplemvp I/TaskRepository: Transaction (insert) at [2129644649860]
07-01 16:38:49.640 3261-3273/com.zhuinden.simplestackdemoexamplemvp I/DatabaseManager: Realm Change occurred [2129651817991]
07-01 16:38:49.644 3261-3273/com.zhuinden.simplestackdemoexamplemvp I/TaskRepository: Opened Realm for tasks with instance count [2] at [2129655345187]
07-01 16:38:49.644 3261-3273/com.zhuinden.simplestackdemoexamplemvp I/TaskRepository: Emit initial tasks [2129655486935]

which is exactly what I need to see for it to work...

If there's one interesting thing though, I used the synchronous API so is it possible that I copied an outdated instance from the transaction on a third thread, even though this is a looper thread?

I guess that is not impossible, maybe using findAllAsync() and isLoaded() instead of immediate emission would help, but that does not explain why change listener wouldn't be called with synchronous API if it was out of date then got updated.

I'll investigate further.

bmeike commented 7 years ago

Hey buddy, what's up with this? Got anything new? Should we close it?

Zhuinden commented 7 years ago

I haven't worked on isolating the problem further, but I still think there is a chance of some strange race condition with multiple looper threads and the synchronous query api.

However, the asynchronous query api handles it right.

I assume because BadVersionException would trigger a new re-evaluation. After all, the synchronous API isn't very natural for the object store.

As changing it to use asynchronous queries in this use-case makes more sense anyways, I am closing the issue.