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

Do not mutate the Realm managed state on the finalizer thread #1222

Merged
merged 1 commit into from
Feb 10, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 30 additions & 19 deletions Shared/Realm.Shared/Realm.cs
Original file line number Diff line number Diff line change
Expand Up @@ -342,7 +342,12 @@ private void Dispose(bool disposing)
return;
}

_state.RemoveRealm(this);
if (disposing)
{
// only mutate the state on explicit disposal
// otherwise we do so on the finalizer thread
_state.RemoveRealm(this);
}
_state = null;

SharedRealmHandle.Close(); // Note: this closes the *handle*, it does not trigger realm::Realm::close().
Expand Down Expand Up @@ -1125,12 +1130,13 @@ public static void NotifyRealmChanged(IntPtr stateHandle)

#endregion

private readonly List<WeakReference> weakRealms = new List<WeakReference>();
private readonly List<WeakReference<Realm>> weakRealms = new List<WeakReference<Realm>>();

public readonly GCHandle GCHandle;

public RealmState()
{
// this is freed in a native callback when the CSharpBindingContext is destroyed
GCHandle = GCHandle.Alloc(this);
}

Expand All @@ -1146,42 +1152,47 @@ public void AddRealm(Realm realm)
{
// We only want to have each realm once. That should be the case as AddRealm
// is only called in the Realm ctor, but let's check just in case.
Debug.Assert(weakRealms.All(r => r.Target as Realm != realm), "Trying to add a duplicate Realm to the RealmState.");
Debug.Assert(!weakRealms.Any(r =>
{
Realm other;
return r.TryGetTarget(out other) && object.ReferenceEquals(realm, other);
}), "Trying to add a duplicate Realm to the RealmState.");

weakRealms.Add(new WeakReference(realm));
weakRealms.Add(new WeakReference<Realm>(realm));
}

public void RemoveRealm(Realm realm)
{
var weakRealm = weakRealms.SingleOrDefault(r => r.Target as Realm == realm);
var weakRealm = weakRealms.SingleOrDefault(r =>
{
Realm other;
return r.TryGetTarget(out other) && object.ReferenceEquals(realm, other);
});
weakRealms.Remove(weakRealm);

if (!weakRealms.Any())
{
realm.SharedRealmHandle.CloseRealm();
GCHandle.Free();
}
}

private IEnumerable<Realm> GetLiveRealms()
{
var shouldPurge = false;
foreach (var weakRealm in weakRealms)
var realms = new List<Realm>();

weakRealms.RemoveAll(r =>
{
if (weakRealm.IsAlive)
Realm realm;
if (r.TryGetTarget(out realm))
{
yield return (Realm)weakRealm.Target;
realms.Add(realm);
return false;
}
else
{
shouldPurge = true;
}
}

if (shouldPurge)
{
weakRealms.RemoveAll(weak => !weak.IsAlive);
}
return true;
});

return realms;
}
}
}
Expand Down
21 changes: 21 additions & 0 deletions Shared/Realm.Shared/native/NativeCommon.cs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,9 @@ internal static class NativeCommon
[return: MarshalAs(UnmanagedType.I1)]
public delegate bool NotifyRealmObjectCallback(IntPtr realmObjectHandle, IntPtr propertyIndex);

[UnmanagedFunctionPointer(CallingConvention.Cdecl)]
public delegate void FreeGCHandleCallback(IntPtr handle);

#if DEBUG
[UnmanagedFunctionPointer(CallingConvention.Cdecl)]
public delegate void DebugLoggerCallback(IntPtr utf8String, IntPtr stringLen);
Expand All @@ -65,6 +68,9 @@ private static unsafe void DebugLogger(IntPtr utf8String, IntPtr stringLen)
[DllImport(InteropConfig.DLL_NAME, EntryPoint = "register_notify_realm_object_changed", CallingConvention = CallingConvention.Cdecl)]
public static extern void register_notify_realm_object_changed(NotifyRealmObjectCallback callback);

[DllImport(InteropConfig.DLL_NAME, EntryPoint = "realm_install_gchandle_deleter", CallingConvention = CallingConvention.Cdecl)]
public static extern void install_gchandle_deleter(FreeGCHandleCallback callback);

[DllImport(InteropConfig.DLL_NAME, EntryPoint = "realm_reset_for_testing", CallingConvention = CallingConvention.Cdecl)]
public static extern void reset_for_testing();

Expand All @@ -83,6 +89,21 @@ public static void Initialize()
GCHandle.Alloc(logger);
set_debug_logger(logger);
#endif

FreeGCHandleCallback gchandleDeleter = FreeGCHandle;
GCHandle.Alloc(gchandleDeleter);
install_gchandle_deleter(gchandleDeleter);
}

#if __IOS__
[MonoPInvokeCallback(typeof(NativeCommon.FreeGCHandleCallback))]
#endif
public static void FreeGCHandle(IntPtr handle)
{
if (handle != IntPtr.Zero)
{
GCHandle.FromIntPtr(handle).Free();
}
}
}
}
15 changes: 14 additions & 1 deletion wrappers/src/shared_realm_cs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@ NotifyRealmChangedDelegate* notify_realm_changed = nullptr;
using NotifyRealmObjectChangedDelegate = bool(void* managed_realm_object_handle, size_t property_ndx);
NotifyRealmObjectChangedDelegate* notify_realm_object_changed = nullptr;

using FreeGCHandleDelegate = void(void* managed_handle);
FreeGCHandleDelegate* free_gc_handle = nullptr;

namespace realm {
namespace binding {
inline size_t get_property_index(const ObjectSchema& schema, const size_t column_index) {
Expand Down Expand Up @@ -116,6 +119,11 @@ namespace binding {
return observer->row_ndx == row_ndx && observer->table_ndx == table_ndx;
});
}

CSharpBindingContext::~CSharpBindingContext()
{
free_gc_handle(m_managed_state_handle);
}
}

}
Expand All @@ -132,7 +140,12 @@ REALM_EXPORT void register_notify_realm_object_changed(NotifyRealmObjectChangedD
{
notify_realm_object_changed = notifier;
}


REALM_EXPORT void realm_install_gchandle_deleter(FreeGCHandleDelegate deleter)
{
free_gc_handle = deleter;
}

REALM_EXPORT SharedRealm* shared_realm_open(Configuration configuration, SchemaObject* objects, int objects_length, SchemaProperty* properties, uint8_t* encryption_key, NativeException::Marshallable& ex)
{
return handle_errors(ex, [&]() {
Expand Down
2 changes: 2 additions & 0 deletions wrappers/src/shared_realm_cs.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,8 @@ namespace binding {
{
return m_observed_rows;
}

~CSharpBindingContext();
private:
void* m_managed_state_handle;
std::vector<BindingContext::ObserverState> m_observed_rows;
Expand Down