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

CsWin32 is effectively unusable with trim/AOT #1273

Closed
Sergio0694 opened this issue Sep 9, 2024 · 11 comments
Closed

CsWin32 is effectively unusable with trim/AOT #1273

Sergio0694 opened this issue Sep 9, 2024 · 11 comments
Labels
invalid This doesn't seem right

Comments

@Sergio0694
Copy link
Member

Sergio0694 commented Sep 9, 2024

Overview

I suspect this is a known issue, but I figured it was about time to open a tracking issue. The generated code that CsWin32 produces is completely incompatible with trim/AOT. This keeps coming up all over the place as more apps and system components start experimenting with NativeAOT or start migrating to it. This applies not just to the default mode for CsWin32, but also to the blittable mode, as it still generates [ComImport] interfaces, as well as P/Invoke-s with unsupported features for eg. disabled runtime marshalling.

The ask is to make the default mode (optionally opt-in, but not just for the blittable mode) to generate code that is trim/AOT-safe. That is, the generated code must be able to compile (when used) with 0 warnings with these two properties being set:

<PublishAot>true</PublishAot>
<DisableRuntimeMarshalling>true</DisableRuntimeMarshalling>

For this to happen, the following things must be true:

  • All generated interfaces must be compatible with ComWrappers
    • Ie. they should be equivalent to interfaces with [GeneratedComInterface] on them
    • All P/Invoke-s should be equivalent to when using [LibraryImport]

Until then, CsWin32 is a non-starter with NativeAOT, and we have to tell developers to not use it.
This is a constant source of frustration, because:

  • Devs keep trying to use CsWin32 because they mistakenly assume it's trim/AOT-safe. It isn't.
  • There are no similar equivalent alternatives.

@jkoritzinsky mentioned it might be possible to update CsWin32 to be built on a shared backend with the COM generators, which is currently work in progress. If so (and if profitable), we should start a conversation/planning to make this happen.

/cc. @manodasanW @jevansaks @Jeremy-Price

@Sergio0694 Sergio0694 added the enhancement New feature or request label Sep 9, 2024
@AArnott
Copy link
Member

AArnott commented Sep 9, 2024

This isn't true. In fact CsWin32 is already heavily used in WinForms which supports trimming.
You just have to set the allowMarshaling setting to false.

@AArnott AArnott closed this as not planned Won't fix, can't repro, duplicate, stale Sep 9, 2024
@AArnott AArnott added invalid This doesn't seem right and removed enhancement New feature or request labels Sep 9, 2024
@AArnott
Copy link
Member

AArnott commented Sep 9, 2024

I don't see us setting CsWin32 to default to generating harder-to-use APIs to support a non-default and minority case of C# users.
Since we already have a setting that supports the non-default runtime mode, what I can see us doing is emitting a warning when AOT is turned on or the marshaler is turned off to advise users to also disable marshaling in the .json file. That would only catch the cases where CsWin32 is used directly in the app project though.

AArnott added a commit that referenced this issue Sep 9, 2024
@AArnott
Copy link
Member

AArnott commented Sep 9, 2024

but also to the blittable mode, as it still generates [ComImport] interfaces, as well as P/Invoke-s with unsupported features for eg. disabled runtime marshalling.

If by blittable mode you mean when allowMarshaling: false has been set, if we still emit code that relies on marshaling in that configuration, please file bugs with specific examples and I'll be happy to fix them.

Until then, CsWin32 is a non-starter with NativeAOT, and we have to tell developers to not use it.

I'd love to get CsWin32 to a place you feel great about recommending its use.

@AArnott
Copy link
Member

AArnott commented Sep 9, 2024

All generated interfaces must be compatible with ComWrappers
Ie. they should be equivalent to interfaces with [GeneratedComInterface] on them
All P/Invoke-s should be equivalent to when using [LibraryImport]

That's a tall order, because roslyn doesn't support chaining source generators. I have asked in the past for the source generator that supports those attributes to pack itself up as a library (as well as its own SU) so that CsWin32 can bundle it and chain it in internally. But so far that hasn't happened AFAIK.

@Sergio0694
Copy link
Member Author

This isn't true. In fact CsWin32 is already heavily used in WinForms which supports trimming.
You just have to set the allowMarshaling setting to false.

It is. Even with allowMarshalling set to false, CsWin32 generates [ComImport] interfaces, which are unusable. Also, WinForms doesn't suppot AOT, which is the main concern here. Not sure I understand how just saying this isn't true helps. Also like I said, it generates P/Invoke-s that rely on runtime marshalling, eg. they have SetLastError = true, which fails if marshalling is disabled.

As of right now, like I said, we had to flat out tell people not to use CsWin32, which isn't great at all.

"I don't see us setting CsWin32 to default to generating harder-to-use APIs to support a non-default and minority case of C# users."

This is not what I asked for at all. I said the default should be to have interfaces, but compatible with ComWrappers.

"That's a tall order, because roslyn doesn't support chaining source generators. I have asked in the past for the source generator that supports those attributes to pack itself up as a library (as well as its own SU) so that CsWin32 can bundle it and chain it in internally. But so far that hasn't happened AFAIK."

@jkoritzinsky mentioned several possible approaches that could be used to address this.

@AArnott
Copy link
Member

AArnott commented Sep 9, 2024

Not sure I understand how just saying this isn't true helps.

Because your opening statement (indeed, the title of the issue) emphatically states something that we have evidence from multiple users and repos to contradict.
Your more precise statements around specific behaviors that are problematic in trimmed/AOT environments are better and appreciated, which is why I suggested you file bugs for each of them.

"I don't see us setting CsWin32 to default to generating harder-to-use APIs to support a non-default and minority case of C# users."

This is not what I asked for at all. I said the default should be to have interfaces, but compatible with ComWrappers.

It sounded to me you also want the default to avoid the runtime marshaler (for non-COM scenarios), which would compromise the API -- for now. If we can get CsWin32 to a point where non-marshaled APIs are as good as the marshaled ones, then changing the default may make sense at that point.
We continue to support .NET Framework targeted projects, and we currently generate very similar APIs across frameworks to ease multi-targeting projects.

@jkoritzinsky mentioned several possible approaches that could be used to address this.

Awesome. I look forward to hearing more.

@Sergio0694
Copy link
Member Author

"Because your opening statement (indeed, the title of the issue) emphatically states something that we have evidence from multiple users and repos to contradict."

How so? I specifically said that this issue is about the fact that:

  • CsWin32 when not in blittable mode is not trim/AOT safe
  • CsWin32 even when in blittable mode, still generates some stuff that is either not trim/AOT-safe (eg. the ComImport interface), or that will fail with disabled runtime marshalling (eg. SetLastError = true on P/Invoke)

I don't know what evidence you have from other repos but both of these things are just objectively true.

I also added that I'm not asking for the default mode to necessarily be changed, but to have at least some option to keep using managed interfaces (ie. not blittable mode), but in a way that's AOT safe. That is, using ComWrappers. It might be true that doing this would be complex, I'm not denying that. But that doesn't mean that what I said is incorrect.

I would love to be able to use CsWin32. But the fact is right now we just had to tell everyone (in Windows) to not use it.

@jkoritzinsky
Copy link
Member

One possible approach that would integrate cleanly with StrategyBasedComWrappers would be to generate the IUnknownDerivedDetailsAttribute on the [ComImport] interface types and provide the implementation types with marshalling (which in the case of "blittable mode" could be as simple as indexing to the function pointer in the vtable and forwarding the arguments). Then StrategyBasedComWrappers would work automatically.

Alternatively, you could define your own model that would work.

I have asked in the past for the source generator that supports those attributes to pack itself up as a library (as well as its own SU) so that CsWin32 can bundle it and chain it in internally. But so far that hasn't happened AFAIK.

I'm hoping to get something like this done soon-ish™️. We'll see if I'm able to.

@AArnott
Copy link
Member

AArnott commented Sep 9, 2024

CsWin32 even when in blittable mode, still generates ... SetLastError = true on P/Invoke

If so, then that's a bug. #1017 should have resolved that long ago and we have a test that proves it still works (at least for the test case). Can you please file an issue with the repro for this, @Sergio0694?

@AArnott
Copy link
Member

AArnott commented Sep 9, 2024

There is a lot about ComWrappers that I don't understand. IIRC the policy we've adopted so far in CsWin32 is to avoid direct support for it, but generate code that is compatible with it. This is how winforms does it: cswin32 + their own COMWrappers glue code.
I vaguely recall some concern that ComWrappers can vary in use (maybe the "strategy" thing that @jkoritzinsky mentioned) and that if cswin32 adopts one particular pattern it may compromise compatibility with other apps. But I really don't know what I'm talking about here and would like to learn more so cswin32 can do the best thing here.

@JeremyKuhne
Copy link
Member

WinForms not fully supporting AOT / Trimming has to do with reflection specific code, not COM. We fully use manual COM in .NET 9. That said we don't expose managed interfaces generated by CsWin32 in any way, so we don't have to deal with that end of things.

Fyi, the key reflection specific code:

  • Data binding
  • BinaryFormatter (embedded resources, some OLE scenarios)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
invalid This doesn't seem right
Projects
None yet
Development

No branches or pull requests

4 participants