From cf989ecf5167f000339598a8851ffb4eab7ffb2a Mon Sep 17 00:00:00 2001 From: Jonathan Pryor Date: Thu, 25 Mar 2021 14:02:49 -0400 Subject: [PATCH] Review and update nullability annotations on ICustomMarshaler Fixes: https://github.com/dotnet/runtime/issues/50144 Re-review the [`ICustomMarshaler` interface][0] in the context of Nullable Reference Types, so that `null` is allowed *everywhere*. Do so because `ICustomMarshaler` is an interface which dates to .NET 1.0, and existing interface implementations already assume that `null` can be used as parameters or return values, including existing unit tests within this repo! ```diff --- a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/ICustomMarshaler.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/ICustomMarshaler.cs @@ -6,17 +6,13 @@ namespace System.Runtime.InteropServices namespace System.Runtime.InteropServices { // This the base interface that must be implemented by all custom marshalers. public interface ICustomMarshaler { - object MarshalNativeToManaged(IntPtr pNativeData); + object? MarshalNativeToManaged(IntPtr pNativeData); - IntPtr MarshalManagedToNative(object ManagedObj); + IntPtr MarshalManagedToNative(object? ManagedObj); void CleanUpNativeData(IntPtr pNativeData); - void CleanUpManagedData(object ManagedObj); + void CleanUpManagedData(object? ManagedObj); int GetNativeDataSize(); } ``` Additional rationale: a cursory [search through GitHub][1] finds numerous examples which return or accept `null`, including: * [`TestLucene.CrapLord.Utf8ConstCustomMarshaler`][2]: allows `MarshalManagedToNative(null)`. * [`Script.DebugMarshaler`][3]: `MarshalNativeToManaged(IntPtr.Zero)` can return `null`, via `Marshal.PtrToStringAuto(IntPtr.Zero)`. * [`FFmpeg.AutoGen.UTF8Marshaler`][4]: returns `null` on `MarshalNativeToManaged(IntPtr.Zero)`, and allows `MarshalManagedToNative(null)`. * [`Microsoft.Win32.Security.SecurityAttributesMarshaler`][5]: returns `null` for `MarshalNativeToManaged(IntPtr.Zero)` and accepts `MarshalManagedToNative(null)`. Updating `ICustomMarshaler` to use `object?` instead of `object` will allow these (and other) existing types to be updated to use C#8 Nullable Reference Types without requiring semantic changes or disabling the nullability checks around `ICustomMarshaler`. [0]: https://docs.microsoft.com/en-us/dotnet/api/system.runtime.interopservices.icustommarshaler?view=net-5.0 [1]: https://github.com/search?q=ICustomMarshaler+marshalnativetomanaged+language%3Acsharp&type=Code [2]: https://github.com/ststeiger/WebRansack/blob/5da604ea4bbc9f35fa35c512b81f8d049fe47ed6/TestLucene/CrapLord/Marshallers/Utf8ConstCustomMarshaler.cs [3]: https://github.com/defin/jrm-code-project/blob/0716e271f449b52bfabe4c2fefe8fc5e62902f42/LScript/DebugMarshaler.cs [4]: https://github.com/jiangguang5201314/VideoRender/blob/b0fc7f172001673138b6bfd193f4ed71f46d4802/AVMedia/AVMedia/FFmpeg/ConstCharPtrMarshaler.cs [5]: https://github.com/dineshkummarc/FlexWikiCore-2.1.0.274/blob/9b83495ac92a2d72a2be63bacfdff0662941fca5/FlexWikiCore-2.1.0.274-Source/lib/Win32Security/src/SecurityAttributesMarshaler.cs --- .../CustomMarshalers/EnumerableToDispatchMarshaler.cs | 6 +++--- .../CustomMarshalers/EnumerableViewOfDispatch.cs | 2 +- .../CustomMarshalers/EnumeratorToEnumVariantMarshaler.cs | 6 +++--- .../CustomMarshalers/ExpandoToDispatchExMarshaler.cs | 6 +++--- .../CustomMarshalers/TypeToTypeInfoMarshaler.cs | 6 +++--- .../src/System/Runtime/InteropServices/ICustomMarshaler.cs | 6 +++--- 6 files changed, 16 insertions(+), 16 deletions(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/Runtime/InteropServices/CustomMarshalers/EnumerableToDispatchMarshaler.cs b/src/coreclr/System.Private.CoreLib/src/System/Runtime/InteropServices/CustomMarshalers/EnumerableToDispatchMarshaler.cs index 3bd63b33612f8..04c5bf4782b4c 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Runtime/InteropServices/CustomMarshalers/EnumerableToDispatchMarshaler.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Runtime/InteropServices/CustomMarshalers/EnumerableToDispatchMarshaler.cs @@ -17,7 +17,7 @@ private EnumerableToDispatchMarshaler() { } - public void CleanUpManagedData(object ManagedObj) + public void CleanUpManagedData(object? ManagedObj) { } @@ -32,7 +32,7 @@ public int GetNativeDataSize() return -1; } - public IntPtr MarshalManagedToNative(object ManagedObj) + public IntPtr MarshalManagedToNative(object? ManagedObj) { if (ManagedObj == null) { @@ -42,7 +42,7 @@ public IntPtr MarshalManagedToNative(object ManagedObj) return Marshal.GetComInterfaceForObject(ManagedObj); } - public object MarshalNativeToManaged(IntPtr pNativeData) + public object? MarshalNativeToManaged(IntPtr pNativeData) { if (pNativeData == IntPtr.Zero) { diff --git a/src/coreclr/System.Private.CoreLib/src/System/Runtime/InteropServices/CustomMarshalers/EnumerableViewOfDispatch.cs b/src/coreclr/System.Private.CoreLib/src/System/Runtime/InteropServices/CustomMarshalers/EnumerableViewOfDispatch.cs index a64da6919c2ed..5bf987d1175cc 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Runtime/InteropServices/CustomMarshalers/EnumerableViewOfDispatch.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Runtime/InteropServices/CustomMarshalers/EnumerableViewOfDispatch.cs @@ -50,7 +50,7 @@ public System.Collections.IEnumerator GetEnumerator() } enumVariantPtr = Marshal.GetIUnknownForObject(enumVariant); - return (System.Collections.IEnumerator)EnumeratorToEnumVariantMarshaler.GetInstance(null).MarshalNativeToManaged(enumVariantPtr); + return (System.Collections.IEnumerator)EnumeratorToEnumVariantMarshaler.GetInstance(null).MarshalNativeToManaged(enumVariantPtr)!; } finally { diff --git a/src/coreclr/System.Private.CoreLib/src/System/Runtime/InteropServices/CustomMarshalers/EnumeratorToEnumVariantMarshaler.cs b/src/coreclr/System.Private.CoreLib/src/System/Runtime/InteropServices/CustomMarshalers/EnumeratorToEnumVariantMarshaler.cs index b4718f5e7c285..33b334074acd3 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Runtime/InteropServices/CustomMarshalers/EnumeratorToEnumVariantMarshaler.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Runtime/InteropServices/CustomMarshalers/EnumeratorToEnumVariantMarshaler.cs @@ -18,7 +18,7 @@ private EnumeratorToEnumVariantMarshaler() { } - public void CleanUpManagedData(object ManagedObj) + public void CleanUpManagedData(object? ManagedObj) { } @@ -33,7 +33,7 @@ public int GetNativeDataSize() return -1; } - public IntPtr MarshalManagedToNative(object ManagedObj) + public IntPtr MarshalManagedToNative(object? ManagedObj) { if (ManagedObj == null) { @@ -50,7 +50,7 @@ public IntPtr MarshalManagedToNative(object ManagedObj) return Marshal.GetComInterfaceForObject(nativeView); } - public object MarshalNativeToManaged(IntPtr pNativeData) + public object? MarshalNativeToManaged(IntPtr pNativeData) { if (pNativeData == IntPtr.Zero) { diff --git a/src/coreclr/System.Private.CoreLib/src/System/Runtime/InteropServices/CustomMarshalers/ExpandoToDispatchExMarshaler.cs b/src/coreclr/System.Private.CoreLib/src/System/Runtime/InteropServices/CustomMarshalers/ExpandoToDispatchExMarshaler.cs index 07e7802e9d2eb..212bcbfd6708e 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Runtime/InteropServices/CustomMarshalers/ExpandoToDispatchExMarshaler.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Runtime/InteropServices/CustomMarshalers/ExpandoToDispatchExMarshaler.cs @@ -13,7 +13,7 @@ private ExpandoToDispatchExMarshaler() { } - public void CleanUpManagedData(object ManagedObj) + public void CleanUpManagedData(object? ManagedObj) { } @@ -27,12 +27,12 @@ public int GetNativeDataSize() return -1; } - public IntPtr MarshalManagedToNative(object ManagedObj) + public IntPtr MarshalManagedToNative(object? ManagedObj) { throw new PlatformNotSupportedException(SR.PlatformNotSupported_IExpando); } - public object MarshalNativeToManaged(IntPtr pNativeData) + public object? MarshalNativeToManaged(IntPtr pNativeData) { throw new PlatformNotSupportedException(SR.PlatformNotSupported_IExpando); } diff --git a/src/coreclr/System.Private.CoreLib/src/System/Runtime/InteropServices/CustomMarshalers/TypeToTypeInfoMarshaler.cs b/src/coreclr/System.Private.CoreLib/src/System/Runtime/InteropServices/CustomMarshalers/TypeToTypeInfoMarshaler.cs index da82567e8c38d..387f0a4dd7dae 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Runtime/InteropServices/CustomMarshalers/TypeToTypeInfoMarshaler.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Runtime/InteropServices/CustomMarshalers/TypeToTypeInfoMarshaler.cs @@ -13,7 +13,7 @@ private TypeToTypeInfoMarshaler() { } - public void CleanUpManagedData(object ManagedObj) + public void CleanUpManagedData(object? ManagedObj) { } @@ -27,12 +27,12 @@ public int GetNativeDataSize() return -1; } - public IntPtr MarshalManagedToNative(object ManagedObj) + public IntPtr MarshalManagedToNative(object? ManagedObj) { throw new PlatformNotSupportedException(SR.PlatformNotSupported_ITypeInfo); } - public object MarshalNativeToManaged(IntPtr pNativeData) + public object? MarshalNativeToManaged(IntPtr pNativeData) { throw new PlatformNotSupportedException(SR.PlatformNotSupported_ITypeInfo); } diff --git a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/ICustomMarshaler.cs b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/ICustomMarshaler.cs index 3fc2c736837c0..9fd08396afa6b 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/ICustomMarshaler.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/ICustomMarshaler.cs @@ -10,13 +10,13 @@ public interface ICustomMarshaler // ILLinker marks all methods of this type equaly so the attribute can be on any of them [System.Diagnostics.CodeAnalysis.DynamicDependency(nameof(Marshal.GetCustomMarshalerInstance), typeof(Marshal))] #endif - object MarshalNativeToManaged(IntPtr pNativeData); + object? MarshalNativeToManaged(IntPtr pNativeData); - IntPtr MarshalManagedToNative(object ManagedObj); + IntPtr MarshalManagedToNative(object? ManagedObj); void CleanUpNativeData(IntPtr pNativeData); - void CleanUpManagedData(object ManagedObj); + void CleanUpManagedData(object? ManagedObj); int GetNativeDataSize(); }