[BUG] InvalidProgramException when [Reactive] property with Nullable<T> type is assigned null in member declaration

[JesperTreetop]

Describe the bug When a Fody-woven [Reactive] property of nullable value type (Nullable<T> for some T) is assigned null directly in the member declaration, running the constructor results in an InvalidProgramException.

Steps To Reproduce

1. Create a new WPF project.
2. Add ReactiveUI and the Fody package.
3. Add a Loaded event handler to the MainWindow window, and make the code mirror:

    public partial class MainWindow : Window {
        public MainWindow() {
            InitializeComponent();
        }

        private void MainWindow_OnLoaded(object sender, RoutedEventArgs e) {
            var z = new TestViewModel(); // <- "A"
        }
    }

    public class TestViewModel :  ReactiveObject {
        [Reactive]
        public int? Floopy { get; set; } = null;
    }

4. Run. The code will crash in location "A". (The event doesn't matter, but the exception would be caught by XAML construction and obscured if run in the window's constructor.)

Expected behavior For there not to be an issue. Assigning null happens to be redundant, and assigning other values does not seem to cause crashes. Nullable reference types are also not affected.

"Just don't do that" is a perfectly reasonable workaround - but it took me an hour to narrow it down to this being the issue.

Environment

• OS: Windows
• Version 10
• Device: PC
• Reactive UI: 13.1.1

[glennawatson]

For v14 of rxui I have a rewritten fody that can handle this much better. Just need time to finish it. Hopefully in the next week or so.

Workaround for the moment is just to use the RaiseAndSetIfChanged for this one

[JesperTreetop]

In my case I solved it by just not assigning null to it, since that's the default value anyway, and since that's the only thing that triggers this bug (that I know of!) it's reasonable. (This appeared in a stretch of code where other properties were assigned to non-default values.) If something else triggered it, it could be fixed by moving the assignment to first thing inside the constructor too, that also works.

[glennawatson]

Our fody doesn't honour default values at the moment that's one of the planned fixes

[glennawatson]

The fody is going into maintenance-only mode at the moment while we work on a new project which will replace the functionality in general. Keeping this open to track for the new project.

[colejohnson66]

This issue was making me tear my hair out the past few days

[colejohnson66]

The issue is how the weaver replaces field assignments with set_X calls in the constructor.

For example, in my program, the weaver was generating this IL:

IL_007f: ldarg.0      // this
IL_0080: call         instance void iDecryptIt.ViewModels.MainWindowViewModel::set_VKGroupSelectedItem(valuetype [System.Runtime]System.Nullable`1<valuetype [iDecryptIt.Shared]iDecryptIt.Shared.DeviceGroup>)
IL_0085: initobj      valuetype [System.Runtime]System.Nullable`1<valuetype [iDecryptIt.Shared]iDecryptIt.Shared.DeviceGroup>

However, an "unweaved" Nullable<T> property such as this:

using System;

public class Program {
    public Nullable<int> Test { get; set; } = null;
}

...generates this IL:

IL_0000: ldarg.0
IL_0001: ldflda valuetype [System.Runtime]System.Nullable`1<int32> Program::'<Test>k__BackingField'
IL_0006: initobj valuetype [System.Runtime]System.Nullable`1<int32>

---

The obvious difference is that the weaver is generating a call opcode to the setter before pushing an actual value onto the stack. The call opcode will pop two values off (this and the argument), but only one was pushed. Then initobj after will pop another two off. So the stack ends up looking like:

// at IL_007f:
  <something1>  <-- TOS
  <something2>
  <something3>
  <something4>

// ldarg.0

// at IL_0080:
  this
  <something1>
  <something2>
  <something3>
  <something4>

// call

// at IL_0085:
  <something2>
  <something3>
  <something4>

// initobj

// after:
  <something4>

Contrast that with what Roslyn generates: a ldflda opcode which pops the two previously pushed values (this and the field). That generates this stack activity:

// at IL_0000:
  <something>

// ldarg.0

// at IL_0001:
  this
  <something>

// ldflda

// at IL_0006:
  address of Program::'<Test>k__BackingField'
  this
  <something>

// initobj

// after:
  <something>

---

This bad generation happens here:

https://github.com/reactiveui/ReactiveUI/blob/f6aa1176ecd3159fe2890cf82d06e5ee452ef936/src/ReactiveUI.Fody/ReactiveUIPropertyWeaver.cs#L128

[colejohnson66]

My guess is: because the Nullable<T> is being assigned with the default value, there's no stfld opcode generated, but the weaver just grabbed the first opcode of any type where the operand is a field reference (which ldflda and stfld both have):

https://github.com/reactiveui/ReactiveUI/blob/f6aa1176ecd3159fe2890cf82d06e5ee452ef936/src/ReactiveUI.Fody/ReactiveUIPropertyWeaver.cs#L115

It seems changing that to also require the opcode to be a stfld might fix the issue.

[colejohnson66]

It seems the compiler special cases Nullable<T> to not generate a stfld opcode when set to null.

In fact, this code:

using System;

public class Program {
    public Nullable<int> Test { get; set; }
    
    public void Main()
    {
        Test = null;
    }
}

// in `Main`
IL_0001: ldarg.0
IL_0002: ldloca.s 0
IL_0004: initobj valuetype [System.Runtime]System.Nullable`1<int32>
IL_000a: ldloc.0
IL_000b: call instance void Program::set_Test(valuetype [System.Runtime]System.Nullable`1<int32>)

In addition, if I modify the property to set it to a non-null value, a stfld opcode is generated (instead of ldflda):

using System;

public class Program {
    public Nullable<int> Test { get; set; } = 0;
}

IL_0000: ldarg.0
IL_0001: ldc.i4.0
IL_0002: newobj instance void valuetype [System.Runtime]System.Nullable`1<int32>::.ctor(!0)
IL_0007: stfld valuetype [System.Runtime]System.Nullable`1<int32> Program::'<Test>k__BackingField'

[KyleC69]

In my case I solved it by just not assigning null to it, since that's the default value anyway, and since that's the only thing that triggers this bug (that I know of!) it's reasonable. (This appeared in a stretch of code where other properties were assigned to non-default values.) If something else triggered it, it could be fixed by moving the assignment to first thing inside the constructor too, that also works.

As of Avalonia 11.0 rc1 and Reactive 18; This bug still exists and also appears when attempting to assign a non-null value to an annotated property. Let me know if more details are needed.

[glennawatson]

Fody are no longer maintained. Look for alternatives hopefully coming soon but we aren't maintaining at the moment due to maintenance burden

[KyleC69]

Thank you for the heads up. That's unfortunate, Avalonia/Reactive may want to remove the recommendations from the docs, so someone else doesn't follow that rabbit hole :) Any plans for Av or Rx to implement annotations?

[glennawatson]

We plan to. Just need time. Be closer to the mvvm toolkit using source generators. Just infinitely easier to maintain.

[KyleC69]

As of RC1 11.0 Source gens seem to still have a problem and requires to still use FindControl, think that might be fixed before releasing 11.0? And thank you for your time and the info. One more new-b question, What is the relationship between AvaloniaUI and ReactiveUI? Is it an off shoot with different maintainers? I find myself flipping between the two trying to solve a problem :) Just curious.

[glennawatson]

No idea. We don't control the Avalonia source generator. Just the rxui model generation stuff

[KyleC69]

Thanks Glenn have a good evening

[github-actions[bot]]

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.