Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Skia Targets allows to change the UI on Background thread #15879

Open
pedrojesus-work opened this issue Mar 14, 2024 · 5 comments · May be fixed by #15925
Open

Skia Targets allows to change the UI on Background thread #15879

pedrojesus-work opened this issue Mar 14, 2024 · 5 comments · May be fixed by #15925
Assignees
Labels
area/skia/stability ✏️ kind/breaking-change 💥 Categorizes an issue or PR as requiring a breaking change to be fixed. kind/bug Something isn't working platform/all Categorizes an issue or PR as relevant to the all platforms

Comments

@pedrojesus-work
Copy link

Current behavior

If I try to edit a TextBox.Text on a Background thread that will work and the text will be updated.

Expected behavior

Some exception saying that I can't change the UI outside the UIThread.

How to reproduce it (as minimally and precisely as possible)

In order to run the issue, just try out this sample. IF you run the Windows target it will fail when the code hits ContinueWith, on Skia targets it will work.
GTKUIBackgroundThread.zip

Workaround

None

Works on UWP/WinUI

Yes

Environment

Uno.WinUI / Uno.WinUI.WebAssembly / Uno.WinUI.Skia

NuGet package version(s)

No response

Affected platforms

No response

IDE

No response

IDE version

No response

Relevant plugins

No response

Anything else we need to know?

No response

@pedrojesus-work pedrojesus-work added kind/bug Something isn't working triage/untriaged Indicates an issue requires triaging or verification difficulty/tbd Categorizes an issue for which the difficulty level needs to be defined. labels Mar 14, 2024
@jeromelaban
Copy link
Member

@pedrojesus-work this is fixed in the latest 5.2 builds, background operations are blocked. Is this a version you tested with?

@pedrojesus-work
Copy link
Author

@jeromelaban, no it's 5.1.87, I'll try the 5.2 and see how that goes

@Youssef1313 Youssef1313 added the triage/potentially-fixed Categorizes an issue as potentially fixed by some unlinked PR, fix needs to be verified label Mar 15, 2024
@pedrojesus-work
Copy link
Author

Changed the sdk version to 5.2.0 "Uno.Sdk": "5.2.0-dev.1921", and still reproduce the error

@jeromelaban
Copy link
Member

@Youssef1313 could you take a look?

@Youssef1313 Youssef1313 self-assigned this Mar 18, 2024
@Youssef1313 Youssef1313 removed the triage/potentially-fixed Categorizes an issue as potentially fixed by some unlinked PR, fix needs to be verified label Mar 18, 2024
@Youssef1313
Copy link
Member

@jeromelaban The added check was in a specific code path that's known to be very problematic when accessed concurrently. It doesn't cover all SetValue calls, but only subset that end up calling DependencyProperty.GetProperty (e.g, it happens for inherited properties for example).

To fix this issue, the check will need to happen in InnerSetValue, but that's a larger breaking change than what was recently introduced. I'll open a PR anyway.

@Youssef1313 Youssef1313 linked a pull request Mar 18, 2024 that will close this issue
6 tasks
@MartinZikmund MartinZikmund added platform/all Categorizes an issue or PR as relevant to the all platforms kind/breaking-change 💥 Categorizes an issue or PR as requiring a breaking change to be fixed. area/skia/stability ✏️ and removed triage/untriaged Indicates an issue requires triaging or verification difficulty/tbd Categorizes an issue for which the difficulty level needs to be defined. labels Jul 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/skia/stability ✏️ kind/breaking-change 💥 Categorizes an issue or PR as requiring a breaking change to be fixed. kind/bug Something isn't working platform/all Categorizes an issue or PR as relevant to the all platforms
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants