Fix: Allow for Control.Invoke execution on Binding updates

[zeh-almeida]

adding option to use Control.Invoke on Binding updates because of UI Thread concurrency exceptions.

Fixes #8582 Fixes #8532

Proposed changes

• Add a constructor parameter to allow Control.Invoke execution
• Adjust the void SetPropValue(object value) to call Control.Invoke when possible
• Adjust the API definition with the new constructor option

Customer Impact

• Will allow multi-thread bindings, which can happen using asynchronous operations.

Regression?

• No

Risk

• Enables a new execution path for bindings

Test methodology

• All previous tests must not be changed
• Added test case for new constructor (TODO)
• Added test case to check for Control.Invoke execution (TODO)

Test environment(s)

.NET SDK: Version: 8.0.100-alpha.1.23061.8 Commit: c8d103ed3c

Runtime Environment: OS Name: Windows OS Version: 10.0.19045 OS Platform: Windows RID: win10-x64

Host: Version: 8.0.0-alpha.1.23079.4 Architecture: x64 Commit: a34291586e

.NET SDKs installed: 8.0.100-alpha.1.23061.8

.NET runtimes installed: Microsoft.AspNetCore.App 8.0.0-alpha.1.23058.7 [G:\projects\zeh-winforms.dotnet\shared\Microsoft.AspNetCore.App] Microsoft.NETCore.App 8.0.0-alpha.1.23057.5 [G:\projects\zeh-winforms.dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 8.0.0-alpha.1.23058.2 [G:\projects\zeh-winforms.dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 8.0.0-alpha.1.23079.4 [G:\projects\zeh-winforms.dotnet\shared\Microsoft.NETCore.App] Microsoft.WindowsDesktop.App 8.0.0-alpha.1.23057.1 [G:\projects\zeh-winforms.dotnet\shared\Microsoft.WindowsDesktop.App]

Other architectures found: x86 [C:\Program Files (x86)\dotnet] registered at [HKLM\SOFTWARE\dotnet\Setup\InstalledVersions\x86\InstallLocation]

Environment variables: Not set

global.json file: .\global.json

Learn more: https://aka.ms/dotnet/info

Download .NET: https://aka.ms/dotnet/download

Microsoft Reviewers: Open in CodeFlow

[dnfadmin]

All CLA requirements met.

[elachlan]

Could you add a test that tests async databinding? Is is possible in stock .NET?

Because this changes the API surface it will require a API Suggestion and review.

[elachlan]

CC: @KlausLoeffelmann if you have a spare moment

[zeh-almeida]

@elachlan as far as I know, there's no async databinding on the APIs. What could happen is a asynchronous operation triggers a binding update, which would run synchronously in relation to this operation.

This is the scenario I came across, which threw an exception because the operation was not running in the UI Thread.

Hope I was clear enough, synchronicity can be mind boggling sometimes.

PS. I am working on adding tests but I am more focused in making sure I don't break anything. Still studying the code.

[elachlan]

Could you please investigate a way to test this? Possibly spawn a thread which triggers the databind update?

[zeh-almeida]

@elachlan I can't, for the life of me, have a successful test run, even on a unmodified branch. I am beggining to think there's something wrong with my setup or some additional software I don't have but I couldn't find anything on the docs.

Is there anyway to make sure?

[elachlan]

Some tests will just fail locally. Don't be too worried about it.

If the tests are related to what you are working on then it's an issue.

[zeh-almeida]

@elachlan added two tests which I have executed successfully in my machine.

[elachlan]

@zeh-almeida Great. So next steps are to get an API review on your initial issue with this PR as an example. I am not a part of the winforms team so I am not 100% sure how that all works. @merriemcgaw might be able to let you know more.

Keep in mind, preview one is around the corner and I imagine the team is pushing to get lots of stuff done before shipping.

[merriemcgaw]

Yep, next steps are documented in the API Review Process page. All .NET API changes go through this process, and they are fairly strict about how the request is formatted so that they can review requests more efficiently. I also would really like @KlausLoeffelmann to sign off on change here as I mentioned in #8532 but I am so excited to see this in use 😄

[KlausLoeffelmann]

Planned to take a look this week.

[ghost]

The current status of this "draft" PR has persisted for over 180 days, making it highly probable that it is no longer aligned with the latest codebase. Our repository is set up to automatically close draft PRs that have become outdated, and it requests the author to revisit and reopen them if they deem it necessary, thereby bringing them to the team's attention.

[zeh-almeida]

@dotnet-policy-service agree

[lonitra]

@KlausLoeffelmann gentle ping to keep this on your radar

[KlausLoeffelmann]

Yes, I have. We are aiming to address this in the early stages of .NET 9.

[KlausLoeffelmann]

If this is something we want to continue working on, could someone rebase this? Thanks!

[zeh-almeida]

I will try to rebase this one this week, would love to have this working for my projects!

[KlausLoeffelmann]

Cool. And take your time. I will not get to it next week, but I will try to make time for it during February. Thanks!

[zeh-almeida]

Hey @KlausLoeffelmann I am working on syncing the code but more than a year's worth of changes do take a lot of time to get right.

I am currently running the necessary tests in my machine and I hope to have this finished until sunday, 18th.

When running .\build -test I get some failing tests though nothing related to my code: apparently is some are about locale? I will debug it further.

Nevertheless, I will update the PR with the current state in order to keep the discussion alive.

[codecov[bot]]

Codecov Report

Attention: 46 lines in your changes are missing coverage. Please review.

Comparison is base (fad8ead) 73.21203% compared to head (e74ef9f) 73.21173%. Report is 2 commits behind head on main.

Additional details and impacted files

@@                 Coverage Diff                 @@
##                main       #8547         +/-   ##
===================================================
- Coverage   73.21203%   73.21173%   -0.00030%     
===================================================
  Files           3081        3081                 
  Lines         633359      633460        +101     
  Branches       47400       47402          +2     
===================================================
+ Hits          463695      463767         +72     
- Misses        166120      166150         +30     
+ Partials        3544        3543          -1     

Flag	Coverage Δ
Debug	73.21173% <61.01695%> (-0.00030%)	:arrow_down:
integration	18.30378% <0.00000%> (-0.00184%)	:arrow_down:
production	46.72078% <70.00000%> (+0.00668%)	:arrow_up:
test	94.98907% <54.41176%> (-0.00767%)	:arrow_down:
unit	43.64696% <70.00000%> (+0.00565%)	:arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

[JeremyKuhne]

@KlausLoeffelmann we need to get updated status in this one and figure out how we can get this closed out.

[zeh-almeida]

Hey there @KlausLoeffelmann and @JeremyKuhne I regret to say that I can no longer contribute to this PR as from the past months as I have responsibilities which I must address first.

Not sure if it is needed but I see no problem in other people taking over this contribution.

[merriemcgaw]

@zeh-almeida thank you so much for letting us know!