oleg-shilo / wixsharp

Framework for building a complete MSI or WiX source code by using script files written with C# syntax.
MIT License
1.08k stars 172 forks source link

(Race condition?) Issue with form load order (difficult to reproduce) #1581

Open smaudet opened 1 month ago

smaudet commented 1 month ago

As the title mentions, there is some issue with the form initialization for wpf UI dialogs.

While they appear to work "fine" under "normal" circumstances, noted that when instrumented some of the eventing appears to be out of order between WixSharp.UIShell.CurrentDialogIndex:175 and WixSharp.UI.WPF\Extensions.cs:363

In order to "fix" this, it is necessary to (somehow) detect the event and remove/re-add the ManagedFormHost from/to the UIShell.shellView. Currently I just have a timer that executes a second afterwards (for the instrumented case). May want to investigate this, attempt to reproduce, and ultimately fix.

Possibly pair down the complexity of the form loading code here as well...although I realize it is partially this complex to support multiple types of UIShell. :)

smaudet commented 1 month ago

This is not a fix, but a patch that can be applied to e.g. WelcomeDialog.xaml.cs

    public void Init()
    {
        ViewModelBinder.Bind(new WelcomeDialogModel { Host = ManagedFormHost }, this, null);
        Task.Run(() =>
        {
            Thread.Sleep(500);
            Dispatcher.Invoke(FormFix);
        });
    }

    private void FormFix()
    {
        if (!(ManagedFormHost.Parent is Form shellView)) {return;}

        shellView.Controls.RemoveAt(0);
        shellView.Controls.Add(ManagedFormHost); // Without this re-add the form does not always correctly populate.
    }

I found http://drwpf.com/blog/2007/10/05/managing-application-resources-when-wpf-is-hosted/ which suggests that perhaps the application is not always fully initialized when wpf is hosted - this did seem to help with resources that I was loading, however did not fix the wpf load issue.

Inspecting the form(s) showed everything as visible, nothing out of the ordinary. I think perhaps some event can be out of order.

The official Microsoft documentation does specify that the control should be created after the hosting is done:

https://learn.microsoft.com/en-us/dotnet/desktop/wpf/advanced/walkthrough-hosting-a-wpf-composite-control-in-windows-forms?view=netframeworkdesktop-4.8

I tried changing the order and it didn't seem to get better. Perhaps there is some other loading error? I'm not sure if there's a way to get winforms to be more verbose about the error.

Inspecting the wpf tree did show that the wpf controls exist, and rendered, even, but do not show up.

oleg-shilo commented 1 month ago

Thank you. I'll investigate.

oleg-shilo commented 1 month ago

Hm... it's a peculiar problem.

however did not fix the wpf load issue.

Maybe the reinsertion (the patch) needs to be done when the visual tree is created/initialized.

Do you have any hint on how to reproduce the "wpf load issue"?

smaudet commented 1 month ago

Do you have any hint on how to reproduce the "wpf load issue"?

Unfortunately, no. I have an instrumentation tool that I must use e.g. on build servers, however it is quite costly (not free), if you want to buy a license I can provide you with that tool.

This tool does a lot of hooking of the UI, so its possible its hooks are interfering and there is no bug without this third party program... maybe if I get some time (because I am curious what it is doing) I could try https://github.com/bgarciaoliveira/InjectHook or https://ntcore.com/files/nthookengine.htm, or even https://github.com/snoopwpf/snoopwpf to try to reproduce the problem conditions...

I should say, it would consistently not work with this tool hooking (the tool starts the msi process chain), whereas it would appear fine without. That to me says "event race condition", although I'm not sure where, coupled with the "fix" and all evidence that in fact it worked "fine" other than the small detail of being completely invisible.

I don't see this type of issue with just winforms, or just wpf, this business so far I've only been able to reproduce with instrumentation+winforms+wpf.

oleg-shilo commented 1 month ago

...on build servers, however, it is quite costly (not free),...

Of course. I understand.

That to me says "event race condition",...

Indeed it looks like an "event race condition".

Your fix, even though it works only under certain circumstances, is a valid workaround. OK, it's not particularly elegant, but when it comes to the MSI everything is not so elegant. Thus I have no problem incorporating your workaround in WixSharp.

However, I am curious if it can be extended to a more comprehensive solution. IE continuously check and apply the fix only when some runtime condition is met (e.g. UIElement visible, VisualTree is loaded). I am asking this because if I understand you right the fix did not solve the problem for your instrumentation tool case.

I propose this: Since I have no means to reproduce the problem I will rely on your suggestion for incorporating your fix in its original or extended form.
No pressure. Even if we maintain the existing status quo without any changes it's still acceptable. The problem seems to be very specific and so far there have been no reports of this sort.


https://github.com/bgarciaoliveira/InjectHook

It brought back some very very distant memories of my C++ time. Hard-core stuff. Hope you don't have to go there...