Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Realm is not refreshed after sync session upload/download callback #5543

Closed
nielsenko opened this issue May 30, 2022 · 9 comments · Fixed by realm/realm-dart#700
Closed

Realm is not refreshed after sync session upload/download callback #5543

nielsenko opened this issue May 30, 2022 · 9 comments · Fixed by realm/realm-dart#700
Assignees

Comments

@nielsenko
Copy link
Contributor

When awaiting download/upload completion the realm is not refreshed after the callback fires. This can cause odd races, where the recently downloaded data is not available when reading the realm.

The fact that waitForDownload doesn't behave as one would expect is kind of unfortunate but we can probably improve things later - probably once we have [refreshAsync implemented in Core (https://github.com//issues/5246). Can we file a ticket to make sure we don't forget though?

Originally posted by @nirinchev in realm/realm-dart#619 (comment)

@fealebenpae
Copy link
Member

I think that's by design. SyncSession::wait_for_upload() doesn't know about any Realm instances that might represent the same file and it can't be influenced by them. To be clear, the sync client does fire a notification on the realm when new data is received, it's just that realm instances aren't guaranteed to have advanced to the latest read transaction inside the wait_for_upload callback. IMO this is something we'll have to handle in the Dart SDK.

@nielsenko
Copy link
Contributor Author

If we had the option of awaiting a refresh asynchronously we could fix this on the dart side.

@cmelchior
Copy link
Contributor

It is by design...you can just manually call SharedRealm::refresh() after it completes (which would make sense for threads that only exist on one thread). This is e.g. what Kotlin does.

On multi-thread SDK's like Java, you cannot do that as it risk updating in the middle of a thread execution

@nielsenko
Copy link
Contributor Author

Also, it will blog my thread. We really need the async version.

@cmelchior
Copy link
Contributor

cmelchior commented May 30, 2022

Couldn't you just use indefinite progresslisteners as a proxy for upload/download? And then call refresh when uploaded = uploaded?

@nirinchev
Copy link
Member

In my original comment, I meant we should file a ticket in the dart SDK. I don't believe Core can do anything about it because the sync client doesn't know which one is the "main thread" Realm to refresh it. It invokes the completion on its own thread and then it's up to the SDK or the framework to dispatch the continuation on the original Realm thread at which point Sync is no longer involved.

Re: progress listeners - they have the same issue - they too originate from the sync thread, so there's no guarantee that when they're invoked, the main thread Realm has refreshed itself.

Re: Kotlin's approach - as @nielsenko said, this is likely a bad idea because us calling refresh() automatically can block the thread for a long time as it will force synchronous evaluation of changes and firing of all notifications listeners the user has registered. So we really need a refresh_async, so let's just file a dart ticket that links to #5246 and wait until it is implemented.

@jbreams
Copy link
Contributor

jbreams commented Jun 6, 2022

Hi all, can you see if this behavior works as expected now? We fixed an issue related to bootstrap's not auto-refreshing properly here #5553 - it was released in v12.1.0 - and I'd like to see if that was the underlying cause of this issue too.

@jbreams
Copy link
Contributor

jbreams commented Jun 23, 2022

@nielsenko - has this been resolved in v12.1.0?

@nielsenko
Copy link
Contributor Author

Sorry about the delay. I have tested with v12.1.0-36-g7b82fcf3 and yes my test is no longer dependent on calling an explicit refresh.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 21, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants