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

Paused sessions seem to keep the Realm open #6372

Closed
nirinchev opened this issue Mar 9, 2023 · 4 comments · Fixed by #6383
Closed

Paused sessions seem to keep the Realm open #6372

nirinchev opened this issue Mar 9, 2023 · 4 comments · Fixed by #6383
Assignees

Comments

@nirinchev
Copy link
Member

I have the following test that fails consistently:

using var realm = Realm.GetInstance(config);

var session = realm.SyncSession;
await session.WaitForUploadAsync();

// Removing the Stop() call here fixes the issue
session.Stop(); // calls SyncSession::pause()

realm.Dispose();

// Even adding a delay doesn't help here
Realm.DeleteRealm(config);

The error being thrown is Cannot delete files of an open Realm. I haven't been able to pinpoint the issue, but what I'm seeing is:

  1. We call SyncSession::pause() which in turn calls SyncSession::do_become_inactive.
  2. When we get to m_sync_manager->unregister_session, we get into auto existing_session = it->second->existing_external_reference(), which unlocks the lock, but doesn't remove the session from the map because someone is holding a reference to the session (this is the SDK).
  3. When the SDK calls realm.Dispose that also closes the managed session, which goes into SyncSession::ExternalReference::~ExternalReference.
  4. SyncSession::did_drop_external_reference calls SyncSession::close, but because we're in Paused state, we unlock the state mutex without unregistering the session from the sync manager.

I'm not sure if the sync client does anything special about paused sessions, but it doesn't appear like it's closing the Realm, which then prevents the deletion.

@sync-by-unito
Copy link

sync-by-unito bot commented Mar 15, 2023

➤ Jonathan Reams commented:

I think I see what's up here. I'm just trying to come up with a good test for it.

@jbreams
Copy link
Contributor

jbreams commented Mar 16, 2023

@nirinchev , I think I've got a fix up in PR #6383 and I've got a test that checks that the ref count on the DBRef is correct, but could you check to see this fixes the issue of being able to remove the file as well, since I think that may be a more platform-specific issue?

@nirinchev
Copy link
Member Author

Sure, I've been distracted with some metrics tests, but will try to do it later today.

@nirinchev
Copy link
Member Author

I can confirm that the changes in #6383 make the .NET test pass consistently 🎉

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.

2 participants