dotnet / wpf

WPF is a .NET Core UI framework for building Windows desktop applications.
MIT License
7.03k stars 1.16k forks source link

OperationPriorityChanged is raised without the new priority #9148

Open Laniusexcubitor opened 3 months ago

Laniusexcubitor commented 3 months ago

Description

The dispatcher hook OperationPriorityChanged is called when the priority has not changed yet on the DispatcherOperation. It is already changed in the PriorityQueue of the dispatcher, but a subscriber of the event has no way of knowing this new priority.

Reproduction Steps

Create a new WPF application
exchange contents of App.xaml.cs with

using System.Diagnostics;
using System.Windows;
using System.Windows.Threading;

namespace WpfApp
{
    public partial class App : Application
    {

        protected override void OnStartup(StartupEventArgs e)
        {
            base.OnStartup(e);

            Dispatcher.Hooks.OperationPriorityChanged += (sender, args) =>
            {
                // Note that args.Operation.Priority is still Inactive here
                Debugger.Break();
            };

            Action action = () => { };
            var operation = Dispatcher.BeginInvoke(action, DispatcherPriority.Inactive);
            operation.Priority = DispatcherPriority.Send;

        }
    }
}

Expected behavior

I would expect the OperationPriorityChanged event to be called, when the priority change was completely finished.

Actual behavior

The OperationPriorityChanged is called when the DispatcherOperation does not reflect the change.

Regression?

I don't think so. This was probably designed this way.

Known Workarounds

No workarounds.

Impact

This is probably an edge case.
I am writing diagnostics for some dispatcher queue issues I am seeing in an application.
This makes it harder to debug. I probably will ignore dispatcher priority changes and just show diagnostics for the original Inactive state.

Configuration

Other information

When changing priority on DispatcherOperation, the operation tells it's dispatcher to move it to the new priority chain. The dispatcher will raise the event, and DispatcherOperation will change it's _priority field after the fact. https://github.com/dotnet/wpf/blob/2b7d210c6669ab8c4779d9d5cc9216b05dff34a8/src/Microsoft.DotNet.Wpf/src/WindowsBase/System/Windows/Threading/DispatcherOperation.cs#L123-L126

miloush commented 3 months ago

Agree this looks like an oversight. Could you use the event trace? It should have the new priority.

Laniusexcubitor commented 3 months ago

Yes, I probably could use the ETW events, but I think that would open up another level of complexity as I also would have to take GC ETW events into account due to the roaming ID of the operation.
But I think I can get my bug fixed with the info I am currently collecting :)

miloush commented 3 months ago

One possible solution would be to add Priority to the DispatcherHookEventArgs, that sounds feasible and backwards compatible.

Let us know what the bug was and what helped you discover it!

Laniusexcubitor commented 3 months ago

Actually, I haven't found the bug yet, and it seems to be gone for now. It's a hang during scrolling, sometimes even a freeze. When I tried debugging, I saw hundreds of items in the dispatcher queue, that's why I started writing PerformanceCounters for the dispatcher queue to find issues we might have. I thought there might be an issue with the queue getting saturated. This triggered me finding this bug. But it turns out, those hundreds of queued operations are all inactive. The scrolling is over a hierarchical treeview like structure and there are hundreds of VirtualizingStackPanels. Each panel will create a DispatcherTimer for cleanup tasks, which will queue an Inactive operation which remains inactive until the DispatcherTimer is ticked. I don't think any number of inactive operations will really degrade the dispatcher queue performance. So it's not a queue issue.

miloush commented 3 months ago

There is a history of issues with virtualized tree views freezing when scrolling, e.g. #1962 or #2490. I don't see any open issue at the moment though, if you have a repro, can you post it into a new issue?

Laniusexcubitor commented 2 months ago

Let us know what the bug was and what helped you discover it!

I finally found the bug in my code two weeks ago. It was some bad piece of code that was spamming actions with Loaded Priority into the dispatcher queue. That piece of code did not run haywire during my last investigations.
But now I could identify it using my dispatcher queue PerformanceCounters. Actually it was quite stupid. Saw the dispatcher queue being overloaded with Loaded actions, and then did a search, where the Loaded priority was used.