realm / realm-dotnet

Realm is a mobile database: a replacement for SQLite & ORMs
https://realm.io
Apache License 2.0
1.25k stars 165 forks source link

RealmInvalidTransactionException when using Compiled Bindings in Xamarin.Forms #1996

Closed robertmiles3 closed 4 years ago

robertmiles3 commented 4 years ago

I recently attempted to enable compiled bindings in my Xamarin.Forms app, but I'm hitting a crash. If enable it on a read-only list, it works fine. However, if I enable it on a list where items in the list are editable (ie Entry) then I get a RealmInvalidTransactionException. I want to use compiled bindings for better performance (see here).

Goals

Enable compiled bindings on a ListView in Xamarin.Forms where the list items are editable Realm objects.

Expected Results

List to function as normal

Actual Results

Realms.Exceptions.RealmInvalidTransactionException: Cannot modify managed objects outside of a write transaction.
  at Realms.NativeException.ThrowIfNecessary (System.Func`2[T,TResult] overrider) [0x0000a] in C:\jenkins\workspace\realm_realm\Realm\Realm\Native\NativeException.cs:57
  at Realms.ObjectHandle.SetString (System.IntPtr propertyIndex, System.String value) [0x00023] in C:\jenkins\workspace\realm_realm\Realm\Realm\Handles\ObjectHandle.cs:240
  at Realms.RealmObject.SetStringValue (System.String propertyName, System.String value) [0x00000] in C:\jenkins\workspace\realm_realm\Realm\Realm\RealmObject.cs:315
  at realmcrash.Models.Item.set_Title (System.String value) [0x00000] in /Users/robertmiles3/Desktop/realmcrash/realmcrash/Models/Item.cs:11
  at realmcrash.Views.FolderDetailPage+<InitializeComponent>_anonXamlCDataTemplate_0.<LoadDataTemplate>typedBindingsM__2 (realmcrash.Models.Item , System.String ) <0x10feb6400 + 0x0007a> in <1b9c1498df8844fd9b15274193199032>:0
  at Xamarin.Forms.Internals.TypedBinding`2[TSource,TProperty].ApplyCore (System.Object sourceObject, Xamarin.Forms.BindableObject target, Xamarin.Forms.BindableProperty property, System.Boolean fromTarget) [0x0019f] in D:\a\1\s\Xamarin.Forms.Core\TypedBinding.cs:229
  at Xamarin.Forms.Internals.TypedBinding`2[TSource,TProperty].Apply (System.Boolean fromTarget) [0x00029] in D:\a\1\s\Xamarin.Forms.Core\TypedBinding.cs:108
  at Xamarin.Forms.BindableObject.SetValueActual (Xamarin.Forms.BindableProperty property, Xamarin.Forms.BindableObject+BindablePropertyContext context, System.Object value, System.Boolean currentlyApplying, Xamarin.Forms.Internals.SetValueFlags attributes, System.Boolean silent) [0x00105] in D:\a\1\s\Xamarin.Forms.Core\BindableObject.cs:457
  at Xamarin.Forms.BindableObject.SetValueCore (Xamarin.Forms.BindableProperty property, System.Object value, Xamarin.Forms.Internals.SetValueFlags attributes, Xamarin.Forms.BindableObject+SetValuePrivateFlags privateAttributes) [0x00173] in D:\a\1\s\Xamarin.Forms.Core\BindableObject.cs:397
  at Xamarin.Forms.Internals.TypedBinding`2[TSource,TProperty].ApplyCore (System.Object sourceObject, Xamarin.Forms.BindableObject target, Xamarin.Forms.BindableProperty property, System.Boolean fromTarget) [0x0011b] in D:\a\1\s\Xamarin.Forms.Core\TypedBinding.cs:218
  at Xamarin.Forms.Internals.TypedBinding`2[TSource,TProperty].Apply (System.Object context, Xamarin.Forms.BindableObject bindObj, Xamarin.Forms.BindableProperty targetProperty, System.Boolean fromBindingContextChanged) [0x00058] in D:\a\1\s\Xamarin.Forms.Core\TypedBinding.cs:135
  at Xamarin.Forms.BindableObject.ApplyBindings (System.Boolean skipBindingContext, System.Boolean fromBindingContextChanged) [0x00041] in D:\a\1\s\Xamarin.Forms.Core\BindableObject.cs:480
  at Xamarin.Forms.BindableObject.SetInheritedBindingContext (Xamarin.Forms.BindableObject bindable, System.Object value) [0x0005a] in D:\a\1\s\Xamarin.Forms.Core\BindableObject.cs:209
  at Xamarin.Forms.Element.SetChildInheritedBindingContext (Xamarin.Forms.Element child, System.Object context) [0x00000] in D:\a\1\s\Xamarin.Forms.Core\Element.cs:470
  at Xamarin.Forms.TemplatedView.SetChildInheritedBindingContext (Xamarin.Forms.Element child, System.Object context) [0x00008] in D:\a\1\s\Xamarin.Forms.Core\TemplatedView.cs:69
  at Xamarin.Forms.Element.<OnBindingContextChanged>b__82_0 (Xamarin.Forms.BindableObject child, System.Object bc) [0x00000] in D:\a\1\s\Xamarin.Forms.Core\Element.cs:308
  at Xamarin.Forms.BindableObjectExtensions.PropagateBindingContext[T] (Xamarin.Forms.BindableObject self, System.Collections.Generic.IList`1[T] children, System.Action`2[T1,T2] setChildBindingContext) [0x0002c] in D:\a\1\s\Xamarin.Forms.Core\BindableObjectExtensions.cs:28
  at Xamarin.Forms.Element.OnBindingContextChanged () [0x00000] in D:\a\1\s\Xamarin.Forms.Core\Element.cs:306
  at Xamarin.Forms.VisualElement.OnBindingContextChanged () [0x00006] in D:\a\1\s\Xamarin.Forms.Core\VisualElement.cs:765
  at Xamarin.Forms.View.OnBindingContextChanged () [0x0000c] in D:\a\1\s\Xamarin.Forms.Core\View.cs:158
  at Xamarin.Forms.ContentView.OnBindingContextChanged () [0x00000] in D:\a\1\s\Xamarin.Forms.Core\ContentView.cs:16
  at Xamarin.Forms.BindableObject.SetInheritedBindingContext (Xamarin.Forms.BindableObject bindable, System.Object value) [0x00062] in D:\a\1\s\Xamarin.Forms.Core\BindableObject.cs:210
  at Xamarin.Forms.Element.SetChildInheritedBindingContext (Xamarin.Forms.Element child, System.Object context) [0x00000] in D:\a\1\s\Xamarin.Forms.Core\Element.cs:470
  at Xamarin.Forms.Element.<OnBindingContextChanged>b__82_0 (Xamarin.Forms.BindableObject child, System.Object bc) [0x00000] in D:\a\1\s\Xamarin.Forms.Core\Element.cs:308
  at Xamarin.Forms.BindableObjectExtensions.PropagateBindingContext[T] (Xamarin.Forms.BindableObject self, System.Collections.Generic.IList`1[T] children, System.Action`2[T1,T2] setChildBindingContext) [0x0002c] in D:\a\1\s\Xamarin.Forms.Core\BindableObjectExtensions.cs:28
  at Xamarin.Forms.Element.OnBindingContextChanged () [0x00000] in D:\a\1\s\Xamarin.Forms.Core\Element.cs:306
  at Xamarin.Forms.Cell.OnBindingContextChanged () [0x00000] in D:\a\1\s\Xamarin.Forms.Core\Cells\Cell.cs:156
  at Xamarin.Forms.BindableObject.BindingContextPropertyChanged (Xamarin.Forms.BindableObject bindable, System.Object oldvalue, System.Object newvalue) [0x0000f] in D:\a\1\s\Xamarin.Forms.Core\BindableObject.cs:500
  at Xamarin.Forms.BindableObject.SetValueActual (Xamarin.Forms.BindableProperty property, Xamarin.Forms.BindableObject+BindablePropertyContext context, System.Object value, System.Boolean currentlyApplying, Xamarin.Forms.Internals.SetValueFlags attributes, System.Boolean silent) [0x00120] in D:\a\1\s\Xamarin.Forms.Core\BindableObject.cs:463
  at Xamarin.Forms.BindableObject.SetValueCore (Xamarin.Forms.BindableProperty property, System.Object value, Xamarin.Forms.Internals.SetValueFlags attributes, Xamarin.Forms.BindableObject+SetValuePrivateFlags privateAttributes) [0x00173] in D:\a\1\s\Xamarin.Forms.Core\BindableObject.cs:397
  at Xamarin.Forms.BindableObject.SetValue (Xamarin.Forms.BindableProperty property, System.Object value, System.Boolean fromStyle, System.Boolean checkAccess) [0x00042] in D:\a\1\s\Xamarin.Forms.Core\BindableObject.cs:334
  at Xamarin.Forms.BindableObject.SetValue (Xamarin.Forms.BindableProperty property, System.Object value) [0x00000] in D:\a\1\s\Xamarin.Forms.Core\BindableObject.cs:311
  at Xamarin.Forms.BindableObject.set_BindingContext (System.Object value) [0x00000] in D:\a\1\s\Xamarin.Forms.Core\BindableObject.cs:41
  at Xamarin.Forms.Internals.TemplatedItemsList`2[TView,TItem].UpdateContent (TItem content, System.Int32 index, System.Object item) [0x00000] in D:\a\1\s\Xamarin.Forms.Core\TemplatedItemsList.cs:614
  at Xamarin.Forms.Internals.TemplatedItemsList`2[TView,TItem].ActivateContent (System.Int32 index, System.Object item) [0x00038] in D:\a\1\s\Xamarin.Forms.Core\TemplatedItemsList.cs:536
  at Xamarin.Forms.Internals.TemplatedItemsList`2[TView,TItem].CreateContent (System.Int32 index, System.Object item, System.Boolean insert) [0x00000] in D:\a\1\s\Xamarin.Forms.Core\TemplatedItemsList.cs:543
  at Xamarin.Forms.Internals.TemplatedItemsList`2[TView,TItem].GetOrCreateContent (System.Int32 index, System.Object item) [0x00023] in D:\a\1\s\Xamarin.Forms.Core\TemplatedItemsList.cs:602
  at Xamarin.Forms.Internals.TemplatedItemsList`2[TView,TItem].get_Item (System.Int32 index) [0x00000] in D:\a\1\s\Xamarin.Forms.Core\TemplatedItemsList.cs:337
  at Xamarin.Forms.Platform.iOS.ListViewRenderer+ListViewDataSource.GetCellForPath (Foundation.NSIndexPath indexPath) [0x00007] in D:\a\1\s\Xamarin.Forms.Platform.iOS\Renderers\ListViewRenderer.cs:1337
  at Xamarin.Forms.Platform.iOS.ListViewRenderer+ListViewDataSource.GetCell (UIKit.UITableView tableView, Foundation.NSIndexPath indexPath) [0x00021] in D:\a\1\s\Xamarin.Forms.Platform.iOS\Renderers\ListViewRenderer.cs:1045
  at at (wrapper managed-to-native) UIKit.UIApplication.UIApplicationMain(int,string[],intptr,intptr)
  at UIKit.UIApplication.Main (System.String[] args, System.IntPtr principal, System.IntPtr delegate) [0x00005] in /Library/Frameworks/Xamarin.iOS.framework/Versions/13.18.3.2/src/Xamarin.iOS/UIKit/UIApplication.cs:86
  at UIKit.UIApplication.Main (System.String[] args, System.String principalClassName, System.String delegateClassName) [0x0000e] in /Library/Frameworks/Xamarin.iOS.framework/Versions/13.18.3.2/src/Xamarin.iOS/UIKit/UIApplication.cs:65
  at realmcrash.iOS.Application.Main (System.String[] args) [0x00001] in /Users/robertmiles3/Desktop/realmcrash/realmcrash.iOS/Main.cs:12## Steps to Reproduce

Code Sample

https://github.com/robertmiles3/realmcrash2

Version of Realm and Tooling

nirinchev commented 4 years ago

Ugh... Yeah, that's not really surprising, but I don't have a good idea how to resolve it. The reason two-way databinding works with Realm is because we do some reflection black magic. Classic bindings in Xamarin Forms use reflection to try and invoke the setters/getters of the properties they are bound to. But before they try to go for the full-blown reflection, they'll inspect the object and see if it implements IReflectableType. RealmObject inheritors do implement it and we give the binding engine a fake setter that will open a transaction, invoke the original setter, then commit the transaction (https://github.com/realm/realm-dotnet/blob/master/Realm/Realm/DataBinding/WovenSetterMethodInfo.cs#L60-L78). We've chosen this approach because: 1) We want transactions to be explicit when invoking the setters from code. Committing a transaction introduces significant overhead when setting a property, so we want to encourage people to think about that and try to batch multiple object updates in a single transaction. So invoking the regular property setters should always be done within a transaction. 2) We understand that a developer doesn't have a lot of control about what the binding engine does and we want to provide a somewhat seamless experience, at the expense of reduced performance for updates made from the binding engine. We're not too worried about the performance there because these updates are typically triggered by a user action and it's unlikely that a user is able to generate more than a few tens of actions per second which is slow enough for Realm to commit transactions without introducing noticeable delay on the main thread (a transaction commit takes between 1 and 5 ms on average). This means that the developer doesn't have to worry about opening a transaction for the UI changes.

So far this approach has worked great - we had a clear distinction between calls from the UI (those would use IReflectableType) and calls from code (which use the regular setters). With compiled bindings though, you're explicitly telling the binding engine what the type is, which means that it now has enough information to invoke the setter directly, meaning it circumvents our IReflectableType magic and makes it impossible for us to identify the setter invocation as coming from the UI.

Now, after all of that context, there are a few mitigations I can think of, none of which is great:

  1. Don't use compiled bindings 😅 Now, I know this may not sound ideal, but when binding to Realm objects, the runtime binding overhead is much smaller than in other cases. This is because Realm's implementation has both type and property caches, which means that after the first invocation for each type, there'll be insignificant overhead from using classic bindings. If you want to speed up bindings at the expense of a slightly longer startup time, you can manually iterate over all object types and force creation of the type info via IReflectableType, which will generate the cache, after which bindings should be nearly as fast as compiled ones. As with all things performance, it's best to measure to verify, but this is by far the easiest option.
  2. Add your own property getters/setters to the models that mimic the IReflectableType fake getters/setters. So something like:
    public class Person : RealmObject
    {
        public string Name { get; set; }
        public string NameUI
        {
            get
            {
                if (this.IsValid)
                {
                    return this.Name;
                }
                return null;
            }
            set
            {
                Transaction writeTransaction = null;
                if (this.Realm != null && !this.Realm.IsInTransaction)
                {
                    writeTransaction = this.Realm.BeginWrite();
                }
                this.Name = value;
                if (writeTransaction != null)
                {
                    writeTransaction.Commit();
                    writeTransaction.Dispose();
                }
            }
        }
    }

    You can then use Name when changing it in code and NameUI for databinding. Obviously this is not great as it introduces a ton of boilerplate, but if the performance gain from using compiled bindings is significant, it might be worth sacrificing maintainability for performance.

  3. This is a build-up on 2, but instead of you writing the boilerplate, we can introduce some convention where if you define a Name and NameUI properties in your model with automatic getter/setter, we'll generate the boilerplate for NameUI to call Name in a transaction. This is just throwing it out there and I don't love it, but we may consider doing something like that to support compiled bindings, although I expect it will take quite a bit of time before we align on an approach there.
robertmiles3 commented 4 years ago

Hmm, gotcha. Thanks for the background info. That makes sense. I agree that none of the approaches are all that hot. However, the particular use-case that I'm needing is only binding 2 properties on the model, so approach 2 isn't horrible. Two questions....

  1. If I choose that route, should that property have an [Ignored] on it so as to not try and persist to the DB? Or is that already implied by having an explicit setter?
  2. Also, I'm guessing this.Realm.BeginWrite(); should be writeTransaction = this.Realm.BeginWrite();?
nirinchev commented 4 years ago
  1. You don't need [Ignored] because you're providing the property implementation and Realm should ignore it.
  2. Yeah, I was writing it by hand and didn't check it super thoroughly, but essentially you want roughly this for the getter and this for the setter.
robertmiles3 commented 4 years ago

Great. I just threw it in, and it works fine. No more crash. Thanks for the tip and so fast! You've been very helpful.

For anyone coming here later with the same issue, I added the following property to my model...

// Reasoning: https://github.com/realm/realm-dotnet/issues/1996
public string DefinitionUI
{
    // https://github.com/realm/realm-dotnet/blob/master/Realm/Realm/DataBinding/WovenGetterMethodInfo.cs#L60-L74
    get => this.IsValid ? this.Definition : null;
    // https://github.com/realm/realm-dotnet/blob/master/Realm/Realm/DataBinding/WovenSetterMethodInfo.cs#L60-L78
    set
    {
        var managingRealm = this?.Realm;
        Transaction writeTransaction = null;
        if (managingRealm != null && !managingRealm.IsInTransaction)
        {
            writeTransaction = managingRealm.BeginWrite();
        }

        this.Definition = value;

        if (writeTransaction != null)
        {
            writeTransaction.Commit();
            writeTransaction.Dispose();
        }
    }
}

...and then changed my binding from Definition to DefinitionUI and added back the x:DataType for the compiled binding.