Skip to content

Commit

Permalink
Review and update nullability annotations on ICustomMarshaler
Browse files Browse the repository at this point in the history
Fixes: dotnet#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
  • Loading branch information
jonpryor committed Mar 25, 2021
1 parent 64da821 commit cf989ec
Show file tree
Hide file tree
Showing 6 changed files with 16 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ private EnumerableToDispatchMarshaler()
{
}

public void CleanUpManagedData(object ManagedObj)
public void CleanUpManagedData(object? ManagedObj)
{
}

Expand All @@ -32,7 +32,7 @@ public int GetNativeDataSize()
return -1;
}

public IntPtr MarshalManagedToNative(object ManagedObj)
public IntPtr MarshalManagedToNative(object? ManagedObj)
{
if (ManagedObj == null)
{
Expand All @@ -42,7 +42,7 @@ public IntPtr MarshalManagedToNative(object ManagedObj)
return Marshal.GetComInterfaceForObject<object, IEnumerable>(ManagedObj);
}

public object MarshalNativeToManaged(IntPtr pNativeData)
public object? MarshalNativeToManaged(IntPtr pNativeData)
{
if (pNativeData == IntPtr.Zero)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ private EnumeratorToEnumVariantMarshaler()
{
}

public void CleanUpManagedData(object ManagedObj)
public void CleanUpManagedData(object? ManagedObj)
{
}

Expand All @@ -33,7 +33,7 @@ public int GetNativeDataSize()
return -1;
}

public IntPtr MarshalManagedToNative(object ManagedObj)
public IntPtr MarshalManagedToNative(object? ManagedObj)
{
if (ManagedObj == null)
{
Expand All @@ -50,7 +50,7 @@ public IntPtr MarshalManagedToNative(object ManagedObj)
return Marshal.GetComInterfaceForObject<EnumVariantViewOfEnumerator, ComTypes.IEnumVARIANT>(nativeView);
}

public object MarshalNativeToManaged(IntPtr pNativeData)
public object? MarshalNativeToManaged(IntPtr pNativeData)
{
if (pNativeData == IntPtr.Zero)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ private ExpandoToDispatchExMarshaler()
{
}

public void CleanUpManagedData(object ManagedObj)
public void CleanUpManagedData(object? ManagedObj)
{
}

Expand All @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ private TypeToTypeInfoMarshaler()
{
}

public void CleanUpManagedData(object ManagedObj)
public void CleanUpManagedData(object? ManagedObj)
{
}

Expand All @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down

0 comments on commit cf989ec

Please sign in to comment.