Make WriteAsync wait for changes asynchronously #1729
Conversation
Realm/Realm/Realm.cs
Outdated
| }).ContinueWith(_ => Refresh(), scheduler); | ||
| }).ContinueWith(_ => | ||
| { | ||
| if (otherVersion == SharedRealmHandle.GetTransactionVersion()) |
There was a problem hiding this comment.
Should this be strict equality? I imagine if the current version is higher (for some reason) than the other version, we should still return?
There was a problem hiding this comment.
Good point, I’ll change the comparison.
Realm/Realm/Realm.cs
Outdated
| RealmChangedEventHandler handler = null; | ||
| handler = (sender, e) => | ||
| { | ||
| ((Realm)sender).RealmChanged -= handler; |
There was a problem hiding this comment.
I'm a bit hazy on the subtleties of doing it like this (via the sender) rather than unsubscribing this.RealmChanged. Is it to avoid capturing this in the closure and what would the harm be if we did capture it?
There was a problem hiding this comment.
Exactly - I didn’t want to capture this and create a reference cycle. If we capture it and the wvent never fires then the realm will leak.
Realm/Realm/Realm.cs
Outdated
| /// Waits for the realm and outstanding objects to be updated to a newer version. | ||
| /// </summary> | ||
| /// <returns>An awaitable <see cref="Task"/>.</returns> | ||
| private Task WaitForChangesAsync() |
There was a problem hiding this comment.
I hope you don't mind a comment from an outsider. Could you consider making this public? And why the change in name compared to Refresh? I would expect it to be called RefreshAsync
There was a problem hiding this comment.
Hmm.. I think I get it. You need to compare the current transaction version number with something, to avoid waiting in vane for a change that may never come. Is it not possible to query the realm for the highest transaction version seen?
|
@tgoyne I added you as a reviewer to give your opinion on the high-level approach and confirm whether it matches what we discussed, thanks. |
nirinchev
force-pushed
the
yg/refresh-async
branch
from
872bd28 to
df3c670
Compare
* c++ portion * Use has_changed rather than the transaction version explicitly * Changelog
nirinchev
force-pushed
the
yg/refresh-async
branch
from
7a98d08 to
246d8d7
Compare
Description
Fixes #1728
TODO