[API Proposal]: New custom type marshaller features for pay-for-play features

[jkoritzinsky]

Background and motivation

After getting some community feedback around the new source generated marshaller, we have decided that we want to add a few additional feature flags that the generator will respect to generate code differently. None of these features require us to change the shape of the marshaller types; the only change in code is emitted by the generator itself.

The NeedsKeepAlive flag will direct source-generated interop to emit a call to keep the marshaller object alive across the native call (though GC.KeepAlive or other means). This feature is used internally in the generator by delegate marshalling and can be used by many other marshallers as well (the internal HandleRefMarshaller comes to mind as well as the many uses of GC.KeepAlive in WinForms.

The RequiresPreservedState feature flag contributes to #69096. Ensuring that we preserve marshaller state between the managed->unmanaged and the unmanaged->managed stages in all scenarios is very expensive, and the majority of marshallers do not require this support. This feature flag provides an opt-in model to preserve state when required (with any additional required overhead) and will direct the generator to emit a diagnostic when preserving state is not supported.

The GuaranteedUnmarshal flag directs the generator to emit the ToManaged call on the marshaller to be generated in the GuaranteedUnmarshal phase in the code-gen (in the finally block in the LibraryImport case). This flag would be used by marshallers like the (currently intrinsic in the marshaller) SafeHandle marshaller that have certain guarantees around unmarshalling to avoid memory leaks if another marshaller throws an exception. Adding this flag (along with RequiresPreservedState should enable us to safely convert the SafeHandle marshalling code to be a public marshaller instead of being intrinsically coded into the generator (which we believe makes the source generator much more maintainable).

API Proposal

namespace System.Runtime.InteropServices.Marshalling;

[Flags]
public enum CustomTypeMarshallerFeatures
{
    None = 0,
    UnmanagedResources = 0x1,
    CallerAllocatedBuffer = 0x2,
    TwoStageMarshalling = 0x4,
+   NeedsKeepAlive = 0x8,
+   RequiresPreservedState = 0x10,
+   GuaranteedUnmarshal = 0x20
}

API Usage

[CustomTypeMarshaller(typeof(TManaged), Features = CustomTypeMarshallerFeatures.NeedsKeepAlive | CustomTypeMarshallerFeatures.RequiresPreservedState | CustomTypeMarshallerFeatures.GuaranteedUnmarshal)]
public struct Marshaller
{
// ... required members
}

Alternative Designs

No response

Risks

We only have a limited number of feature flags possible with the 32-bit CustomTypeMarshallerFeatures enum, so we need to make sure we don't use up all possible options.

Tagging subscribers to this area: @dotnet/interop-contrib
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Background and motivation

After getting some community feedback around the new source generated marshaller, we have decided that we want to add a few additional feature flags that the generator will respect to generate code differently. None of these features require us to change the shape of the marshaller types; the only change in code is emitted by the generator itself.

The NeedsKeepAlive flag will direct source-generated interop to emit a call to keep the marshaller object alive across the native call (though GC.KeepAlive or other means). This feature is used internally in the generator by delegate marshalling and can be used by many other marshallers as well (the internal HandleRefMarshaller comes to mind as well as the many uses of GC.KeepAlive in WinForms.

The RequiresPreservedState feature flag contributes to #69096. Ensuring that we preserve marshaller state between the managed->unmanaged and the unmanaged->managed stages in all scenarios is very expensive, and the majority of marshallers do not require this support. This feature flag provides an opt-in model to preserve state when required (with any additional required overhead) and will direct the generator to emit a diagnostic when preserving state is not supported.

The GuaranteedUnmarshal flag directs the generator to emit the ToManaged call on the marshaller to be generated in the GuaranteedUnmarshal phase in the code-gen (in the finally block in the LibraryImport case). This flag would be used by marshallers like the (currently intrinsic in the marshaller) SafeHandle marshaller that have certain guarantees around unmarshalling to avoid memory leaks if another marshaller throws an exception. Adding this flag (along with RequiresPreservedState should enable us to safely convert the SafeHandle marshalling code to be a public marshaller instead of being intrinsically coded into the generator (which we believe makes the source generator much more maintainable).

API Proposal

namespace System.Runtime.InteropServices.Marshalling;

[Flags]
public enum CustomTypeMarshallerFeatures
{
    None = 0,
    UnmanagedResources = 0x1,
    CallerAllocatedBuffer = 0x2,
    TwoStageMarshalling = 0x4,
+   NeedsKeepAlive = 0x8,
+   RequiresPreservedState = 0x10,
+   GuaranteedUnmarshal = 0x20
}

API Usage

[CustomTypeMarshaller(typeof(TManaged), Features = CustomTypeMarshallerFeatures.NeedsKeepAlive | CustomTypeMarshallerFeatures.RequiresPreservedState | CustomTypeMarshallerFeatures.GuaranteedUnmarshal)]
public struct Marshaller
{
// ... required members
}

Alternative Designs

No response

Risks

We only have a limited number of feature flags possible with the 32-bit CustomTypeMarshallerFeatures enum, so we need to make sure we don't use up all possible options.

Author:	jkoritzinsky
Assignees:	-
Labels:	api-suggestion, area-System.Runtime.InteropServices, source-generator
Milestone:	-

[AaronRobinsonMSFT]

+ NeedsKeepAlive = 0x8,

Perhaps we could ask the marshaller author to provide a KeepAlive() implementation instead of asking us to do the work.

[jkotas]

RequiresPreservedState

Would it make sense to deal with this situation by doing the state-free marshalling using static methods? I think it is confusing to allow a state that is not guaranteed to be preserved.

GuaranteedUnmarshal

Why is it not enough to use general cleanup (FreeNative method) for this?

[jkoritzinsky]

RequiresPreservedState

Would it make sense to deal with this situation by doing the state-free marshalling using static methods? I think it is confusing to allow a state that is not guaranteed to be preserved.

The majority of marshallers fall into the scenario where the need state preserved only between the From/ToNativeValue and the corresponding pair (constructor or ToManaged method) and the FreeNative method. This flag is for preserving state between the constructor all the way to the ToManaged method. I guess we could do something state-free for the majority of the marshallers, but we don't have a design for that at the moment.

GuaranteedUnmarshal

Why is it not enough to use general cleanup (FreeNative method) for this?

The code to support "guaranteed unmarshal" through FreeNative would be very odd. We'd have to return the managed value in ToManaged, but still store it in the marshaller so we can update it with the actual values from native in the FreeNative method. Additionally, I don't know if it would work for the unmanaged->managed stub direction, which we're starting with in the COM-focused generators soon. (Although, SafeHandle marshalling doesn't work in the unmanaged->managed direction, so I guess we don't necessarily need to support it if our main/only use case is not applicable in that scenario anyway)

[jkotas]

I guess we could do something state-free for the majority of the marshallers, but we don't have a design for that at the moment.

Yes, I think it would help with clarity. The state management in the current marshaller API shape is confusing. It makes it look like that the state is always preserved for the whole transaction, but it is not actually the case. Can we change the design such that marshaller instance state (if there is any) is always preserved for the whole transaction, and adjust the meaning name of the RequiresPreservedState flag accordingly?

[JeremyKuhne]

One scenario I would like to make sure works for NeedsKeepAlive is when we're marshalling a non-blittable struct. Specifically thinking of HandleRef or equivalent. Would still like a non-allocating way to tag managed objects that we don't want finalized. I don't know if there is a generic, non-allocating way to "root" a struct with managed fields?

[jkoritzinsky]

GC.KeepAlive has an optimization now in the JIT where it won't box a struct. Instead it will mark the struct value in a different way that will keep it alive.