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

.Net 8 upgrade is not able send WM_DESTROY message to ActiveXControl from interop assembly #12551

Open
Sachin-NI opened this issue Nov 27, 2024 · 10 comments · Fixed by Sachin-NI/winforms#1, #12564 or #12648
Assignees
Labels
💥 regression-release Regression from a public release 🚧 work in progress Work that is current in progress

Comments

@Sachin-NI
Copy link
Contributor

Sachin-NI commented Nov 27, 2024

.NET version

net8.0-windows

Did it work in .NET Framework?

Yes

Did it work in any of the earlier releases of .NET Core or .NET 5+?

Issue description

Parent container control is not sending WM_DESTROY message to the child control while disposing the ActiveX child controls and thus our custom onDestroy method never get called.

From my call stack, I could pin point the cause of this issue to be this change by @JeremyKuhne in DetachAndForward Method in AxHost.cs.
The issue is that isHandleCreated need to be stored before calling DetachWindow() method in the DetachAndForward method as we were doing earlier, because isHandleCreated value will always get set to false after calling DetachWindow method so the code inside the if block will never be executed in any case.

New Code after the changes done in this PR:

// New Code
private unsafe void DetachAndForward(ref Message m)
{
    DetachWindow();
    if (IsHandleCreated)
    {
        void* wndProc = (void*)PInvoke.GetWindowLong(this, WINDOW_LONG_PTR_INDEX.GWL_WNDPROC);
        m.ResultInternal = PInvoke.CallWindowProc(
            (delegate* unmanaged[Stdcall]<HWND, uint, WPARAM, LPARAM, LRESULT>)wndProc,
            HWND,
            (uint)m.Msg,
            m.WParamInternal,
            m.LParamInternal);

        GC.KeepAlive(this);
    }
}

Old Code before this PR:

// Old Code
private HWND GetHandleNoCreate() => IsHandleCreated ? (HWND)Handle : default;
private void DetachAndForward(ref Message m)
{
    IntPtr handle = GetHandleNoCreate();
    DetachWindow();
    if (handle != IntPtr.Zero)
    {
        IntPtr wndProc = User32.GetWindowLong(new HandleRef(this, handle), User32.GWL.WNDPROC);
        m.Result = User32.CallWindowProcW(wndProc, handle, (User32.WM)m.Msg, m.WParam, m.LParam);
    }
}

As we are going to release our product with .net 8, We need the fix to be also present in .net 8.

Steps to reproduce

OR

  1. Create a Parent Container Control:

    • Implement a parent container control that hosts ActiveX child controls.
    • Ensure the parent control has the necessary logic to create and manage the lifecycle of the child controls.
  2. Add ActiveX Child Controls:

    • Add one or more ActiveX child controls to the parent container.
    • Ensure these child controls have custom OnDestroy methods that should be called when they are destroyed.
  3. Implement Custom OnDestroy Method:

    • In the ActiveX child controls, implement a custom OnDestroy method that performs specific cleanup tasks.
    • Ensure this method is mapped to the WM_DESTROY message using the MESSAGE_HANDLER macro.
  4. Trigger Disposal of Child Controls:

    • Implement a mechanism in the parent container to dispose of the child controls. This could be triggered by a user action or a specific event in the application.
  5. Observe Message Handling:

    • Use debugging tools or logging to observe the messages sent to the child controls during the disposal process.
    • Specifically, check if the WM_DESTROY message is being sent to the child controls.
  6. Verify OnDestroy Method Execution:

    • Verify whether the custom OnDestroy method in the child controls is being called.
@Sachin-NI Sachin-NI added the untriaged The team needs to look at this issue in the next triage label Nov 27, 2024
Sachin-NI referenced this issue Nov 27, 2024
We still had a number of potential and actual overflow scenarios. In order to make this more resilient, move SendMessage, PostMessage, and Get/SetWindowLong from IntPtr to nint.

This also makes things slightly faster as we aren't creating IntPtr structs and invoking the cast methods on it.

Note that IntPtr implicitly converts to nint. I've removed most of the actual IntPtr usages. I've also removed most of the HandleRef overloads and vetted the callers to do the right thing in regards to avoiding finalizer issues.

There is more we can do here, but this is a big first step.

Also clean up DateTimePicker and move SYSTEMTIME converters to SYSTEMTIME.

I've tried to remove all (IntPtr) casts in SendMessage calls.
@elachlan elachlan added the 💥 regression-release Regression from a public release label Nov 27, 2024
@dotnet-policy-service dotnet-policy-service bot removed the untriaged The team needs to look at this issue in the next triage label Nov 28, 2024
@dotnet-policy-service dotnet-policy-service bot added the 🚧 work in progress Work that is current in progress label Nov 28, 2024
@Sachin-NI Sachin-NI reopened this Nov 28, 2024
@JeremyKuhne JeremyKuhne self-assigned this Dec 2, 2024
@lonitra
Copy link
Member

lonitra commented Dec 2, 2024

Reopening for pending work to add test + servicing

@lonitra lonitra reopened this Dec 2, 2024
@Sachin-NI
Copy link
Contributor Author

@lonitra / @JeremyKuhne , do you have an expected date for patching .NET 8.0 with this change? We urgently need this fix in .NET 8, as our product release depends on it and this issue is affecting our product quality.

@JeremyKuhne
Copy link
Member

@Sachin-NI we need to get approval for it and validate the fix. If it is approved, it would be in February most likely.

@Syareel-Sukeri
Copy link
Contributor

@Sachin-NI Could you provide a sample solution with the custom OnDestroy method in the child controls to verify its execution during disposal?

@Sachin-NI
Copy link
Contributor Author

@Syareel-Sukeri,
I'm not able to attach the zip file in the comment hence I've uploaded it in my branch.
Download the zip file and you'll find 2 solution files in it. One is creating ActiveX COM control which also has WM_DESTROY msg handler and the other solution is importing that control in winForm and sending WM_DESTROY msg while disposing the form.
To reproduce the issue, first build (In Admin VS mode) NIComControl.sln and then build NIFormCOM.sln and launch the generated exe.
If OnDestroy method get called, then you would see "WM_DESTROY received" message pop-up. This is not happening currently, after this PR, this will be resolved.
Note: Make sure you build same configuration for both solutions.

@lonitra lonitra reopened this Dec 17, 2024
@Tanya-Solyanik
Copy link
Member

Tanya-Solyanik commented Dec 18, 2024

@LeafShi1 - please prepare backport PRs to release/8.0 and 9.0, when Syareel signs off on the fix, include both PRs attached to this issue. @JeremyKuhne will add tests to the servicing PRs later.

@Syareel-Sukeri
Copy link
Contributor

Syareel-Sukeri commented Dec 18, 2024

@lonitra @Tanya-Solyanik There is an exception currently blocking the verification process when using binaries built from main branch of Winforms repo to verify. I will continue to verify again when all changes flow into SDK.
Image

@lonitra
Copy link
Member

lonitra commented Dec 18, 2024

@Syareel-Sukeri System.Private.Windows.GdiPlus is a new assembly added recently and should produce when building from main. Are you getting this exception if that dll is included?

@Syareel-Sukeri
Copy link
Contributor

@lonitra Yes, I encounter the exception regardless of whether the System.Private.Windows.GdiPlus.dll is included.

@Syareel-Sukeri
Copy link
Contributor

Syareel-Sukeri commented Dec 19, 2024

@lonitra @Tanya-Solyanik Reverted to the previous branch before System.Private.Windows.GdiPlus.dll was created and manually applied the code changes in AxHost.cs as recommended by Tanya. Built the branch and placed the generated DLLs in the .NET 10.0.100-alpha.1.24617.8 SDK. Verified the issue, and the parent control now correctly sends the WM_DESTROY message to the child controls during the disposal of ActiveX child controls.
Image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
💥 regression-release Regression from a public release 🚧 work in progress Work that is current in progress
Projects
None yet
6 participants