Fix: Allow for Control.Invoke execution on Binding updates #8547
Conversation
…f UI Thread concurrency exceptions.
|
|
|
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. |
|
CC: @KlausLoeffelmann if you have a spare moment |
|
@elachlan as far as I know, there's no async databinding on the APIs. 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. |
|
Could you please investigate a way to test this? Possibly spawn a thread which triggers the databind update? |
|
@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? |
|
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. |
|
@elachlan added two tests which I have executed successfully in my machine. |
|
@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. |
|
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 😄 |
|
Planned to take a look this week. |
ghost
closed this
|
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. |
|
@dotnet-policy-service agree |
|
@KlausLoeffelmann gentle ping to keep this on your radar |
|
Yes, I have. We are aiming to address this in the early stages of .NET 9. |
lonitra
added
the
waiting-on-team
|
If this is something we want to continue working on, could someone rebase this? |
|
I will try to rebase this one this week, would love to have this working for my projects! |
|
Cool. And take your time. I will not get to it next week, but I will try to make time for it during February. |
|
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 Nevertheless, I will update the PR with the current state in order to keep the discussion alive. |
Codecov ReportAttention:
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
Flags with carried forward coverage won't be shown. Click here to find out more. |
|
@KlausLoeffelmann we need to get updated status in this one and figure out how we can get this closed out. |
|
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. |
|
@zeh-almeida thank you so much for letting us know! |
adding option to use
Control.Invokeon Binding updates because of UI Thread concurrency exceptions.Fixes #8582
Fixes #8532
Proposed changes
Control.Invokeexecutionvoid SetPropValue(object value)to callControl.Invokewhen possibleCustomer Impact
Regression?
Risk
Test methodology
Control.Invokeexecution (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