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

Prevent random dispatcher deadlocks #14229

Merged
merged 2 commits into from
Feb 6, 2024

Conversation

ltetak
Copy link
Contributor

@ltetak ltetak commented Jan 16, 2024

What does the pull request do?

It happens from time to time that the rendering stops (tested on macOS but probably can happen on any platform). The culprit seems to be the Dispatcher does not keep properly the info if the impl has been signaled or not.

What is the current behavior?

_signaled property is true but the native implementation does nothing which breaks the loop

What is the updated/expected behavior with this PR?

don't break the loop

How was the solution implemented (if it's not obvious)?

I could not reproduce it 100% reliably but this seemed like the only place and it also makes no sense (to me) to reset the flag here since it was not signaled to the native impl.

Checklist

@avaloniaui-bot
Copy link

You can test this PR using the following package version. 11.1.999-cibuild0043638-beta. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

@maxkatz6 maxkatz6 requested a review from kekekeks January 17, 2024 04:24
@kekekeks
Copy link
Member

kekekeks commented Feb 2, 2024

  1. the problem is most certainly elsewhere
  2. this change will cause perf issues

Please, provide a repro app, since it might be app-specific.

@ltetak
Copy link
Contributor Author

ltetak commented Feb 2, 2024

@kekekeks 100% repro steps. On any platform (tested on windows + mac)

  1. Open Avalonia solution + ControlCatalog project
  2. Open Dispatcher.Queue.cs
  3. Make sure CanQueryPendingInput is false (override on windows, mac is false already - that makes it platform specific)
  4. put breakpoint on line 195 + run
    https://github.com/ltetak/Avalonia/blob/9f5a1027fd109d59b5519b5146c834e7b5c50668/src/Avalonia.Base/Threading/Dispatcher.Queue.cs#L195
    This will make the condition true (because of the delay). And set the _signaled to true.
  5. continue debugging - the Dispatcher is dead now

@kekekeks
Copy link
Member

kekekeks commented Feb 3, 2024

Hmmm, it seems that it's a leftover code from the time when foreground and background processing shared the same state flag. So now requesting background processing in that particular scenario effectively disables the foreground one.

@maxkatz6 maxkatz6 added the backport-candidate-11.0.x Consider this PR for backporting to 11.0 branch label Feb 3, 2024
@maxkatz6 maxkatz6 added this pull request to the merge queue Feb 6, 2024
Merged via the queue into AvaloniaUI:master with commit ece2e69 Feb 6, 2024
6 checks passed
maxkatz6 pushed a commit that referenced this pull request Feb 8, 2024
@maxkatz6 maxkatz6 added backported-11.0.x and removed backport-candidate-11.0.x Consider this PR for backporting to 11.0 branch labels Feb 8, 2024
@cfnfy
Copy link

cfnfy commented Feb 29, 2024

@ltetak - Thank you so much for finding and fixing this issue!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants