Winforms: control.DataBindings.Remove\Clear does not unsubscribe from the INotifyPropertyChanged.PropertyChanged event in the WinForms app.

[NatKite]

Hello!

I think that this is the compatibility bug. (.NetCore 3.0 preview8)
Build and run the code below at the ,Net Framework and at the .Net Core platforms and we will see a difference.

using System;
using System.ComponentModel;
using System.Windows.Forms;

namespace TestApp {
    class Program {
        public class MainObject : INotifyPropertyChanged {
            private string text;
            public string Text {
                get { return text; }
                set {
                    if(text != value) {
                        text = value;
                        if(PropertyChanged != null) {
                            PropertyChanged(this, new PropertyChangedEventArgs(nameof(Text)));
                        }
                    }
                }
            }

            public event PropertyChangedEventHandler PropertyChanged;

            public bool IsPropertyChangedAssigned { get { return PropertyChanged != null; } }

        }

        static void Main(string[] args) {
            MainObject mainObject = new MainObject();
            mainObject.Text = "Test text";
            Form form = new Form();
            TextBox textBox = new TextBox();
            Binding binding = new Binding("Text", mainObject, "Text");
            textBox.DataBindings.Add(binding);
            textBox.Parent = form;
            form.Show();
            Console.WriteLine("MainObject.IsPropertyChangedAssigned: {0}", mainObject.IsPropertyChangedAssigned);
            textBox.DataBindings.Clear();
            binding = null;
            Console.WriteLine("MainObject.IsPropertyChangedAssigned: {0}", mainObject.IsPropertyChangedAssigned);
            Console.ReadLine();
        }
    }
}

For .Net Framework the result is:
MainObject.IsPropertyChangedAssigned: True
MainObject.IsPropertyChangedAssigned: False
For .Net Core the result is:
MainObject.IsPropertyChangedAssigned: True
MainObject.IsPropertyChangedAssigned: True

The PropertyChanged event subscription remains after binding remove.

Thanks,
Nat.

[hughbe]

cref https://github.com/dotnet/winforms/issues/625

[RussKie]

The referenced issue in CoreFx was presumably addressed back in April '19 in dotnet/corefx#34465.
However the bug still repros in WinForms 3.0 P9

[IgorShirinsky]

We have a similar problem with overriden PropertyDescriptor containing additional internal subscribtion. The internal handler still has a subscribtion after removal the binding from the DataBindings collection. It seems like the this condition https://github.com/dotnet/winforms/blob/dc6f4d50aefeb4a45ef605c5c57bc2f403a3999f/src/System.Windows.Forms/src/System/Windows/Forms/Binding.cs#L1161 does not execute because the BindingManagerBase has been set to null earlier. https://github.com/dotnet/winforms/blob/dc6f4d50aefeb4a45ef605c5c57bc2f403a3999f/src/System.Windows.Forms/src/System/Windows/Forms/Binding.cs#L188

[RussKie]

This looks like a regression introduced in #854. I rolled to the previous version of the Binding class and it worked as expected.
/cc: @hughbe

[RussKie]

I managed to narrow down the regression to this change (trimmed for brewety but retained the gist):

         internal void SetListManager(BindingManagerBase newBindingManagerBase)
         {
@@ -1086,14 +1090,13 @@ namespace System.Windows.Forms

         internal class BindToObject
         {
-            private BindingManagerBase _bindingManager;
             private readonly Binding _owner;
             private bool _dataSourceInitialized = false;
             private bool _waitingOnDataSource = false;

             private void PropValueChanged(object sender, EventArgs e)
             {
-                _bindingManager?.OnCurrentChanged(EventArgs.Empty);
+                _owner.BindingManagerBase?.OnCurrentChanged(EventArgs.Empty);
             }

             private bool IsDataSourceInitialized
@@ -1154,19 +1157,19 @@ namespace System.Windows.Forms

             internal void SetBindingManagerBase(BindingManagerBase lManager)
             {
-                if (_bindingManager == lManager)
+                if (_owner.BindingManagerBase == lManager)
                 {
                     return;
                 }

                 // remove notification from the backEnd
-                if (_bindingManager != null && FieldInfo != null && _bindingManager.IsBinding && !(_bindingManager is CurrencyManager))
+                if (_owner.BindingManagerBase != null && FieldInfo != null && _owner.BindingManagerBase.IsBinding && !(_owner.BindingManagerBase is CurrencyManager))
                 {
-                    FieldInfo.RemoveValueChanged(_bindingManager.Current, new EventHandler(PropValueChanged));
+                    FieldInfo.RemoveValueChanged(_owner.BindingManagerBase.Current, new EventHandler(PropValueChanged));
                     FieldInfo = null;
                 }

-                _bindingManager = lManager;
+                _owner.BindingManagerBase = lManager;
                 CheckBinding();
             }

It appears we made an incorrect assumption that because _bindingManager and _owner.BindingManagerBase of the same type they are the same. They are not.
An instance of BindToObject keeps track of its own binding manager that maintains the bindings for the object in question.

[hughbe]

Has this been merged into master?

[RussKie]

No, 3.1 only so far.
I'll port it over

[RussKie]

The port to the master - https://github.com/dotnet/winforms/pull/1963

[Lydia-Shi]

Verified this bug with .Net core 3.1.100-preview1-14076 from release branch, the issue has been fixed.