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

Calls to comctl32.dll succeed in .NET 4.8, but fail in .NET 5 with System.EntryPointNotFoundException #6810

Open
augustoproiete opened this issue Nov 14, 2020 · 28 comments
Milestone

Comments

@augustoproiete
Copy link

  • .NET Core Version: 5.0.100 and 3.1.404
  • Windows version: Windows 10 (18363.1139)
  • Does the bug reproduce also in WPF for .NET Framework 4.8?: No
  • Is this bug related specifically to tooling in Visual Studio (e.g. XAML Designer, Code editing, etc...)? No

Problem description:

Calls to comctl32.dll succeed in .NET Framework 4.8, but fail in .NET 5 and also in .NET Core 3.1 with a System.EntryPointNotFoundException.

e.g.

[DllImport("comctl32.dll", PreserveSig = false)]
private static extern void TaskDialogIndirect([In] ref TaskDialogConfig pTaskConfig, out int pnButton,
    out int pnRadioButton, [MarshalAs(UnmanagedType.Bool)] out bool pfVerificationFlagChecked);

// ...

var config = new TaskDialogConfig
{
    pszWindowTitle = "Task dialog title",
    pszMainInstruction = "Task dialog main instruction",
    dwCommonButtons = TaskDialogCommonButtonFlags.OkButton | TaskDialogCommonButtonFlags.CancelButton,
    // ...
};

using (new ComCtlv6ActivationContext(enable: true))
{
    TaskDialogIndirect(ref config, out _, out _, out _);
}

Actual behavior:

image

System.EntryPointNotFoundException: Unable to find an entry point named 'TaskDialogIndirect' in DLL 'comctl32.dll'.
   at TaskDialogApi.TaskDialog.TaskDialogIndirect(TaskDialogConfig& pTaskConfig, Int32& pnButton, Int32& pnRadioButton, Boolean& pfVerificationFlagChecked)

Expected behavior:

Display the Task Dialog requested

image

Minimal repro:

@augustoproiete
Copy link
Author

augustoproiete commented Nov 14, 2020

This seems to be a similar issue to dotnet/runtime#42593 and dotnet/sdk#12904, and the workaround is the same as described by @vitek-karas on dotnet/runtime#42593 (comment) with the difference that the exception occurs regardless of how the application is run or published.

  • ❌ publishing the app as self-contained (non-single-file) -> fails without the custom manifest
  • ❌ publishing the app as self-contained single-file -> fails without the custom manifest
  • ❌ publishing the app as framework dependent (non-single-file) -> fails without the custom manifest
  • ❌ publishing the app as framework dependent single-file -> fails without the custom manifest

@vitek-karas
Copy link
Member

The workaround is going to be the same, but the underlying issue is probably very different. dotnet/runtime#42593 is about WinForms which has code to enable comctl32 APIs on app start. WPF doesn't seem to have that code. I don't know why it works on .NET Framework - the application doesn't have the needed section in its manifest there either. Somebody from WPF should look into this and determine how it worked in .NET Framework and how/if should work in .NET Core.

@ryalanms
Copy link
Member

@augustoproiete: Is this a general .NET Core issue or a WPF issue?

@augustoproiete
Copy link
Author

augustoproiete commented Nov 17, 2020

@ryalanms I would say this is a WPF-specific issue because in Windows Forms (.NET Core) the error above doesn't happen.

@ryalanms
Copy link
Member

@SamBent will look at the manifest loading for .NET Framework to see why this works there. Thanks.

@SamBent
Copy link
Contributor

SamBent commented Nov 24, 2020

The app tries to load the required manifest itself, here. This code looks for a file named "XPThemes.manifest" next to mscorlib.dll, whose location it determines by typeof(object).Assembly.Location. You're seeing three cases:

  1. In .NET Framework, the manifest is found (in "C:\Windows\Microsoft.NET\Framework\v4.0.30319\XPThemes.manifest"), and the P/Invoke into comctl32.dll succeeds.
  2. In .NET Core + WinForms, the manifest is not found, but WinForms has loaded an equivalent one in initialization. The P/Invoke succeeds.
  3. In .NET Core + WPF, the manifest is not found. The P/Invoke fails with EntryPointNotFoundException. [I get the same failure in .NET Framework by changing the name to "XPThemes.manifestXXX".]

This is an app bug, not WPF. The app assumes something that is true in .NET Framework but not in .NET Core, namely that "XPThemes.manifest" lives next mscorlib.dll. Whether that assumption should hold is a question for .NET runtime; WPF has no stake, and no reason to load an equivalent manifest the way WinForms does.

@SamBent SamBent closed this as completed Nov 24, 2020
@vatsan-madhavan
Copy link
Member

vatsan-madhavan commented Nov 24, 2020

In .NET Framework, XPThemes.manifest was supplied by either WinForms or the runtime in a location adjacent to mscorlib.dll.

In .NET Core, WindowsDesktop framework forgot to provide XPThemes.manifest adjacent to mscorlib.dll - this is a compat break between .NET Framework and .NET Core.

Whether WinForms or runtime makes it available again is a good question to discuss and resolve. I recommend moving this issue to winforms rather than just closing it.

/cc @dotnet/dotnet-winforms

@augustoproiete
Copy link
Author

@SamBent Thank you for the detailed analysis! The repro code is adapted from Microsoft's WindowsAPICodePack (which no longer exists - http://code.msdn.microsoft.com/WindowsAPICodePack/) and I can only assume they expected the XPThemes.manifest file to change over time (and thus didn't embed it in the library).

It would be great if the WinForms team or runtime team could make the XPThemes.manifest file available again in a future version of .NET 5 or .NET 6. I'm happy to send a PR if needed.

Thank you @vatsan-madhavan for trying to get this fixed. I hope this gets re-opened and transferred to the appropriate team.

@vitek-karas
Copy link
Member

WinForms does carry the manifest in .NET Core, but it's a resource in the winforms dll. You can see how WinForms solved a problem around single-file, by extracting the that resource to disk and loading it as a manifest - https://github.com/dotnet/winforms/pull/4149/files#diff-4bb832ea247560f6b393cd7989713d833469e8b70c47961b086a88b5345439aeR834-R838.

@SamBent SamBent reopened this Nov 24, 2020
@SamBent SamBent transferred this issue from dotnet/wpf Nov 24, 2020
@vatsan-madhavan
Copy link
Member

WinForms does carry the manifest in .NET Core, but it's a resource in the winforms dll.

That seems to have solved WinForms' own internal needs, but didn't account for the fact that this manifest may have been used by apps (like this example). That said, these are hard to foresee use-cases, I think.

The ref-pack might be a good place to put this. It may even be reasonable to say that this is not supported in .NET 5/6 and developers would have to supply their own manifest, as long as we document how the manifest is to be used.

That said, I'd recommend a different design altogether. Both WPF and WinForms have occasional (but fairly critical) need for application manifests - there are lots of things in WPF that are controlled in app manifest (notably high DPI related). You should about and enumerate all the scenarios that require app.manifests to be used, and (a) think of ways to do those things in code if it makes sense (for e.g., can an applications can declare in XAML or in C# it's DPI affinity instead of having to resort to app.manifest) and (b) for the items that really do require app.manifest -> make the WindowsDesktop SDK auto-generate an app manifest during application build-process with the right elements. This only applies when an app.manifest is not supplied by the developer, but it ought to solve a bunch of corner-case problems that devs have today with WPF/WinForms.

@RussKie
Copy link
Member

RussKie commented Nov 25, 2020

Sorry I don't quite understand why the issue is transferred to us. Perhaps I'm missing something, is there something which makes Windows Forms to break WPF? Unless it is the case I don't see anything that we need to fix here - Windows Forms apps work, it is WPF that is broken.

If this issue needs to be fixed at WindowsDesktop level the issue should be transferred there. Other than that I think the WPF team needs to provide a fix.

/cc: @merriemcgaw

@kirsan31
Copy link

@RussKie I think that guys here want XPThemes.manifest to be delivered separately. For now, wpf completely haven't it and in winforms it's embedded in winforms.dll.

@merriemcgaw
Copy link
Member

@vatsan-madhavan, @SamBent is there a reason why WPF can't or shouldn't embed this themselves? Or could WPF do this in the WindowsDesktop repo rather than WinForms changing how we're delivering the manifest?

@vatsan-madhavan
Copy link
Member

vatsan-madhavan commented Dec 4, 2020

This is not about embedded manifests, it's about loose manifests.

A future fix could be done by any party in the WindowsDesktop repo ecosystem - you are absolutely correct.

I was simply pointing out that this (comctl32 compat manifest) has been something historically provided by WinForms (or maybe it was provided by BCL), and that this looks like a compat miss from netfx -> netcore port in 3.0.

WPF normally isn't focused on loading Win32 controls or ComCtl32 (modulo many many exceptions ;-)) or enabling those scenarios as a first-class experience for developers (whereas WinForms is focused on exactly those things) - so philosophically this aligns better with WinForms IMHO.

@RussKie
Copy link
Member

RussKie commented Dec 5, 2020

Thank you @vatsan-madhavan for sharing your thoughts, though I am still unconvinced the resolution lies with us. By making changes we may regress our experiences, and we will become responsible for owning WPF experiences (including single file bundling, etc.). This feels as a misplaced responsibility, as we lack necessary expertise and capacity to do this.

Moving the issue to the WindowsDesktop.

@RussKie RussKie transferred this issue from dotnet/winforms Dec 5, 2020
@merriemcgaw
Copy link
Member

@ryalanms I'm interested in your thoughts and @JeremyKuhne when he's back from vacation. Obviously we want to get this working for WPF customers.

@merriemcgaw
Copy link
Member

WinForms team basically feels that it is not efficient for WinForms to provide this loose manifest - we've always (even in .NET Framework) used the embedded manifest approach. In other words we don't know how WPF used the loose manifest and we believe that WPF should find a solution either here with a loose manifest approach or embedding the manifest from resources like WinForms does.

@ryalanms
Copy link
Member

ryalanms commented Jan 8, 2021

Sorry, @merriemcgaw. I haven't been able to look at this yet, but it sounds like WPF should embed the manifest to get parity with .NET Framework.

@ryalanms ryalanms transferred this issue from dotnet/windowsdesktop Jan 8, 2021
@SamBent SamBent transferred this issue from dotnet/wpf Jan 8, 2021
@SamBent
Copy link
Contributor

SamBent commented Jan 8, 2021

@ryalanms @merriemcgaw - I disagree. This is not a WPF issue, but rather a "non-WinForms" issue. Calling comctl32 is not a WPF feature, and WPF has never "used the loose manifest" or done anything else to either enable or disable it. It's something a .NET app can choose to do on its own, independent of whether the app uses WPF (or WinForms, or any other framework). Embedding the manifest in WPF would not solve the problem for apps that don't use either WPF or WinForms. The solution has to live at a deeper level.

In .NET Fx, apps expect to find the loose manifest next to mscorlib. I don't know who added it to the .NET install, but it was almost certainly not WPF. Its affinity to mscorlib suggests that CLR added it, and its independence from WPF and WinForms suggests that the core runtime is the right place to solve the problem (whether by embedding the manifest or by some other means).

@vitek-karas
Copy link
Member

I have no idea who added the manifest in .NET Framework, I just want to clarify that the fact that it lives next to mscorlib is probably not very telling. There are more than 400 files next to mscorlib in .NET Framework...

That said I think this is something which should ideally work "seamlessly" whenever "Windows Dektop" is asked for. For example, it might make sense to automatically include this in the application manifest for any app which targets net5-windows (or net6-windows).

@vatsan-madhavan
Copy link
Member

Until WindowsDesktop framework was introduced into .NET Core, the incentive to load comctl32 in .NET Core applications was minimal - Common Controls are Windows specific and GUI centric. If Windows Forms has no problems loading ComCtl32, then is this effectively a WPF problem ? I think so, but @SamBent are you saying that there is a larger case to be made here that runtime folks should solve it because the affected domain is really (much) larger than WPF (not counting Windows Forms) ?

Given this issue is not getting closed out, I'm going to make a provisional assumption that everybody thinks that this should be solved (and not ignored/rejected) like @vitek-karas said.

@SamBent
Copy link
Contributor

SamBent commented Jan 8, 2021

@vatsan-madhavan I'm just saying that the affected domain is (.NET apps that want to use ComCtl32) - (WinForms apps). I make no claims as to whether that set is equal to (WPF apps). Even if that were true today, there's no guarantee it will be true in the future. Thus it's prudent to treat this as a "non-WinForms" problem rather than as a WPF problem, and solve it at a deeper level than WPF.

Also, it's possible that other cases will arise of .NET apps that want to use some technology unknown to WPF or WinForms or any specific .NET subframework/component, requiring a manifest or similar app-level enabler. It shouldn't be WPF's job to solve these cases. Set the right precedent now.

@Tanya-Solyanik
Copy link
Member

it might make sense to automatically include this in the application manifest for any app which targets net5-windows (or net6-windows).

@vitek-karas - sounds like WPF, WinForms and runtime are agreeing that this manifest should be provided on a level below the specific UI technology, would it make sense to move the bug to the runtime repo?

@vitek-karas
Copy link
Member

Well - it's already in runtime repo right now 😉
Depending on the design it might not belong there. For example if we were to implement the solution where windows apps get different manifest by default, it would probably belong to the SDK repo.

But I think there's a discussion to have:

  • We want to preserve backward compat with .NET Framework -> we would have to include XPTheme.manifest in the shared framework (next to coreclr.dll and other "root" assemblies). This doesn't feel right to be honest, since we would have to do this for all apps, not just net5-windows apps - any app on windows regardless of its purpose would get that file.
  • We want to fix the experience for new apps -> we can tie it to the TFM and make SDK use different default manifest based on the TFM (net5 -> no change, net5-windows -> the new manifest) - but there are probably other similar solutions.

@ghost
Copy link

ghost commented Jan 24, 2021

Tagging subscribers to this area: @vitek-karas, @agocke
See info in area-owners.md if you want to be subscribed.

Issue Details
  • .NET Core Version: 5.0.100 and 3.1.404
  • Windows version: Windows 10 (18363.1139)
  • Does the bug reproduce also in WPF for .NET Framework 4.8?: No
  • Is this bug related specifically to tooling in Visual Studio (e.g. XAML Designer, Code editing, etc...)? No

Problem description:

Calls to comctl32.dll succeed in .NET Framework 4.8, but fail in .NET 5 and also in .NET Core 3.1 with a System.EntryPointNotFoundException.

e.g.

[DllImport("comctl32.dll", PreserveSig = false)]
private static extern void TaskDialogIndirect([In] ref TaskDialogConfig pTaskConfig, out int pnButton,
    out int pnRadioButton, [MarshalAs(UnmanagedType.Bool)] out bool pfVerificationFlagChecked);

// ...

var config = new TaskDialogConfig
{
    pszWindowTitle = "Task dialog title",
    pszMainInstruction = "Task dialog main instruction",
    dwCommonButtons = TaskDialogCommonButtonFlags.OkButton | TaskDialogCommonButtonFlags.CancelButton,
    // ...
};

using (new ComCtlv6ActivationContext(enable: true))
{
    TaskDialogIndirect(ref config, out _, out _, out _);
}

Actual behavior:

image

System.EntryPointNotFoundException: Unable to find an entry point named 'TaskDialogIndirect' in DLL 'comctl32.dll'.
   at TaskDialogApi.TaskDialog.TaskDialogIndirect(TaskDialogConfig& pTaskConfig, Int32& pnButton, Int32& pnRadioButton, Boolean& pfVerificationFlagChecked)

Expected behavior:

Display the Task Dialog requested

image

Minimal repro:

Author: augustoproiete
Assignees: -
Labels:

area-Host, untriaged

Milestone: -

@AaronRobinsonMSFT
Copy link
Member

@vitek-karas and @agocke I am moving this over to the Host side. I am doing this because this is about setting up manifests for specific hosted scenarios and not really related to any interoperability concern in the runtime.

@agocke
Copy link
Member

agocke commented Jul 14, 2022

It sounds like the only compatible solution is to provide the manifest directly next to corlib, even for non-Windows Desktop apps. That doesn't sound like an acceptable solution to me. I'd rather take the compat break. As far as fixing the experience in new apps, I'll leave that up to the WindowsDesktop framework authors.

For now, this is Won't Fix.

@agocke agocke closed this as completed Jul 14, 2022
@RussKie
Copy link
Member

RussKie commented Jul 18, 2022

Reopening and moving to dotnet/wpf for the WPF team to triage because this issue is only affecting WPF apps.

@RussKie RussKie reopened this Jul 18, 2022
@RussKie RussKie removed the area-Host label Jul 18, 2022
@RussKie RussKie transferred this issue from dotnet/runtime Jul 18, 2022
@karelz karelz added this to the Future milestone Sep 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

No branches or pull requests