Merge pull request #177 from CesiumGS/safehandles

Use SafeHandles instead of raw IntPtrs to hold native implementation instances
CesiumGS · Jan 29, 2023 · cb21853 · cb21853
2 parents c14a122 + e13ee3e
commit cb21853
Show file tree

Hide file tree

Showing 35 changed files with 97 additions and 143 deletions.
diff --git a/Reinterop~/CSharpType.cs b/Reinterop~/CSharpType.cs
@@ -7,18 +7,22 @@ internal class CSharpType
         public readonly CppGenerationContext Context;
         public readonly InteropTypeKind Kind;
         public readonly IReadOnlyCollection<string> Namespaces;
-        public readonly ITypeSymbol Symbol;
+        public readonly string Name;
+        public readonly SpecialType SpecialType;
+        public readonly ITypeSymbol? Symbol;
 
         public Compilation Compilation
         {
             get { return Context.Compilation; }
         }
 
-        public CSharpType(CppGenerationContext context, InteropTypeKind kind, IReadOnlyCollection<string> namespaces, ITypeSymbol symbol)
+        public CSharpType(CppGenerationContext context, InteropTypeKind kind, IReadOnlyCollection<string> namespaces, string name, SpecialType specialType, ITypeSymbol? symbol = null)
         {
             this.Context = context;
             this.Kind = kind;
             this.Namespaces = new List<string>(namespaces);
+            this.Name = name;
+            this.SpecialType = specialType;
             this.Symbol = symbol;
         }
 
@@ -38,31 +42,41 @@ public static CSharpType FromSymbol(CppGenerationContext context, ITypeSymbol sy
 
             namespaces.Reverse();
 
-            return new CSharpType(context, kind, namespaces, symbol);
+            return new CSharpType(context, kind, namespaces, symbol.Name, symbol.SpecialType, symbol);
         }
 
         public string GetFullyQualifiedNamespace()
         {
-            IArrayTypeSymbol? arraySymbol = this.Symbol as IArrayTypeSymbol;
-            if (arraySymbol != null)
-                return CSharpType.FromSymbol(this.Context, arraySymbol.ElementType).GetFullyQualifiedNamespace();
+            if (this.Symbol != null)
+            {
+                IArrayTypeSymbol? arraySymbol = this.Symbol as IArrayTypeSymbol;
+                if (arraySymbol != null)
+                    return CSharpType.FromSymbol(this.Context, arraySymbol.ElementType).GetFullyQualifiedNamespace();
+                else
+                    return Symbol.ContainingNamespace.ToDisplayString();
+            }
             else
-                return Symbol.ContainingNamespace.ToDisplayString();
+            {
+                return string.Join(".", this.Namespaces);
+            }
         }
 
         public string GetFullyQualifiedName()
         {
-            return Symbol.ToDisplayString();
+            if (this.Symbol != null)
+                return this.Symbol.ToDisplayString();
+            else
+                return this.GetFullyQualifiedNamespace() + "." + this.Name;
         }
 
         private CSharpType AsInteropTypeCommon()
         {
             // C++ doesn't specify the size of a bool, and C# uses different sizes in different contexts.
             // So we explicitly marshal bools as uint8_t / System.Byte.
-            if (this.Symbol.SpecialType == SpecialType.System_Boolean)
+            if (this.SpecialType == SpecialType.System_Boolean)
                 return CSharpType.FromSymbol(Context, Compilation.GetSpecialType(SpecialType.System_Byte));
             else if (this.Kind == InteropTypeKind.ClassWrapper || this.Kind == InteropTypeKind.NonBlittableStructWrapper || this.Kind == InteropTypeKind.Delegate)
-                return new CSharpType(Context, InteropTypeKind.Primitive, new string[] { "System" }, Compilation.GetSpecialType(SpecialType.System_IntPtr));
+                return CSharpType.FromSymbol(Context, Compilation.GetSpecialType(SpecialType.System_IntPtr));
             else
                 return this;
         }
@@ -100,7 +114,7 @@ public CSharpType AsInteropTypeReturn()
         /// </summary>
         public string GetConversionToInteropType(string variableName)
         {
-            if (this.Symbol.SpecialType == SpecialType.System_Boolean)
+            if (this.SpecialType == SpecialType.System_Boolean)
                 return $"{variableName} ? (byte)1 : (byte)0";
             else if (this.Kind == InteropTypeKind.ClassWrapper || this.Kind == InteropTypeKind.NonBlittableStructWrapper || this.Kind == InteropTypeKind.Delegate)
                 return $"Reinterop.ObjectHandleUtility.CreateHandle({variableName})";
@@ -114,7 +128,7 @@ public string GetConversionToInteropType(string variableName)
 
         public string GetParameterConversionFromInteropType(string variableName)
         {
-            if (this.Symbol.SpecialType == SpecialType.System_Boolean)
+            if (this.SpecialType == SpecialType.System_Boolean)
                 return $"{variableName} != 0";
             else if (this.Kind == InteropTypeKind.ClassWrapper || this.Kind == InteropTypeKind.NonBlittableStructWrapper || this.Kind == InteropTypeKind.Delegate)
                 return $"({this.GetFullyQualifiedName()})Reinterop.ObjectHandleUtility.GetObjectFromHandle({variableName})!";
@@ -128,7 +142,7 @@ public string GetParameterConversionFromInteropType(string variableName)
 
         public string GetReturnValueConversionFromInteropType(string variableName)
         {
-            if (this.Symbol.SpecialType == SpecialType.System_Boolean)
+            if (this.SpecialType == SpecialType.System_Boolean)
                 return $"{variableName} != 0";
             else if (this.Kind == InteropTypeKind.ClassWrapper || this.Kind == InteropTypeKind.NonBlittableStructWrapper || this.Kind == InteropTypeKind.Delegate)
                 return $"({this.GetFullyQualifiedName()})Reinterop.ObjectHandleUtility.GetObjectAndFreeHandle({variableName})!";
@@ -157,7 +171,9 @@ public static bool IsFirstDerivedFromSecond(ITypeSymbol first, ITypeSymbol secon
 
         public CSharpType AsPointer()
         {
-            return new CSharpType(this.Context, InteropTypeKind.Primitive, this.Namespaces, this.Compilation.CreatePointerTypeSymbol(this.Symbol));            
+            if (this.Symbol == null)
+                return this;
+            return new CSharpType(this.Context, InteropTypeKind.Primitive, this.Namespaces, this.Symbol.Name, this.Symbol.SpecialType, this.Compilation.CreatePointerTypeSymbol(this.Symbol));            
         }
     }
 }
diff --git a/Reinterop~/CodeGenerator.cs b/Reinterop~/CodeGenerator.cs
@@ -94,7 +94,7 @@ public static void WriteCSharpCode(GeneratorExecutionContext context, CppGenerat
                 if (partialMethods == null || partialMethods.Methods.Count == 0)
                     continue;
 
-                context.AddSource(partialMethods.Type.Symbol.Name + "-generated", partialMethods.ToSourceFileString());
+                context.AddSource(partialMethods.Type.Name + "-generated", partialMethods.ToSourceFileString());
             }
         }
 

diff --git a/Reinterop~/CustomDelegateGenerator.cs b/Reinterop~/CustomDelegateGenerator.cs
@@ -83,7 +83,7 @@ private void GenerateDelegate(CppGenerationContext context, GeneratedResult resu
                 genericTypeHash = Interop.HashParameters(null, named.TypeArguments);
             }
 
-            string csBaseName = $"{csType.GetFullyQualifiedNamespace().Replace(".", "_")}_{csType.Symbol.Name}{genericTypeHash}_CreateDelegate";
+            string csBaseName = $"{csType.GetFullyQualifiedNamespace().Replace(".", "_")}_{csType.Name}{genericTypeHash}_CreateDelegate";
             string invokeCallbackName = $"{csType.GetFullyQualifiedNamespace().Replace(".", "_")}_{item.Type.Name}{genericTypeHash}_InvokeCallback";
             string disposeCallbackName = $"{csType.GetFullyQualifiedNamespace().Replace(".", "_")}_{item.Type.Name}{genericTypeHash}_DisposeCallback";
 
@@ -107,16 +107,16 @@ private void GenerateDelegate(CppGenerationContext context, GeneratedResult resu
                 CSharpName: csBaseName + "Delegate",
                 CSharpContent:
                     $$"""
-                    private class {{csType.Symbol.Name}}{{genericTypeHash}}NativeFunction : System.IDisposable
+                    private class {{csType.Name}}{{genericTypeHash}}NativeFunction : System.IDisposable
                     {
                         private IntPtr _callbackFunction;
 
-                        public {{csType.Symbol.Name}}{{genericTypeHash}}NativeFunction(IntPtr callbackFunction)
+                        public {{csType.Name}}{{genericTypeHash}}NativeFunction(IntPtr callbackFunction)
                         {
                             _callbackFunction = callbackFunction;
                         }
 
-                        ~{{csType.Symbol.Name}}{{genericTypeHash}}NativeFunction()
+                        ~{{csType.Name}}{{genericTypeHash}}NativeFunction()
                         {
                             Dispose(false);
                         }
@@ -139,7 +139,7 @@ private void Dispose(bool disposing)
                         public {{csReturnType.GetFullyQualifiedName()}} Invoke({{string.Join(", ", invokeParameters)}})
                         {
                             if (_callbackFunction == null)
-                                throw new System.ObjectDisposedException("{{csType.Symbol.Name}}");
+                                throw new System.ObjectDisposedException("{{csType.Name}}");
                     
                             {{csResultImplementation}}{{invokeCallbackName}}({{string.Join(", ", callInvokeInteropParameters)}});
                             {{csReturnImplementation}};
@@ -156,7 +156,7 @@ private void Dispose(bool disposing)
                     [AOT.MonoPInvokeCallback(typeof({{csBaseName}}Type))]
                     private static unsafe IntPtr {{csBaseName}}(IntPtr callbackFunction)
                     {
-                        var receiver = new {{csType.Symbol.Name}}{{genericTypeHash}}NativeFunction(callbackFunction);
+                        var receiver = new {{csType.Name}}{{genericTypeHash}}NativeFunction(callbackFunction);
                         return Reinterop.ObjectHandleUtility.CreateHandle(new {{csType.GetFullyQualifiedName()}}(receiver.Invoke));
                     }
                     """

diff --git a/Reinterop~/GeneratedCSharpPartialMethodDefinitions.cs b/Reinterop~/GeneratedCSharpPartialMethodDefinitions.cs
@@ -40,24 +40,48 @@ public GeneratedCSharpPartialMethodDefinitions(CSharpType type)
 
         public string ToSourceFileString()
         {
+            if (Type.Symbol == null)
+                throw new Exception("Type with partial method definitions must have a Symbol.");
+
             // TODO: support structs
             string kind = "class";
 
+            string interopPrefix = Type.GetFullyQualifiedName().Replace(".", "_");
+            if (!string.IsNullOrEmpty(this.Type.Context.BaseNamespace))
+                interopPrefix = this.Type.Context.BaseNamespace + "_" + interopPrefix;
+
             if (needsInstance)
             {
                 return
                     $$"""
+                    using Microsoft.Win32.SafeHandles;
                     using System;
+                    using System.Runtime.ConstrainedExecution;
                     using System.Runtime.InteropServices;
 
                     namespace {{Type.GetFullyQualifiedNamespace()}}
                     {
                         {{CSharpTypeUtility.GetAccessString(Type.Symbol.DeclaredAccessibility)}} partial {{kind}} {{Type.Symbol.Name}} : System.IDisposable
                         {
+                            internal class ImplementationHandle : SafeHandleZeroOrMinusOneIsInvalid
+                            {
+                                public ImplementationHandle({{Type.Symbol.Name}} managed) : base(true)
+                                {
+                                    SetHandle({{interopPrefix}}_CreateImplementation(Reinterop.ObjectHandleUtility.CreateHandle(managed)));
+                                }
+                    
+                                [ReliabilityContract(Consistency.WillNotCorruptState, Cer.Success)]
+                                protected override bool ReleaseHandle()
+                                {
+                                    {{interopPrefix}}_DestroyImplementation(this.handle);
+                                    return true;
+                                }
+                            }
+                    
                             [System.NonSerialized]
-                            private System.IntPtr _implementation = System.IntPtr.Zero;
+                            private ImplementationHandle _implementation = null;
 
-                            public IntPtr NativeImplementation
+                            internal ImplementationHandle NativeImplementation
                             {
                                 get { return _implementation; }
                             }

diff --git a/Reinterop~/Interop.cs b/Reinterop~/Interop.cs
@@ -326,7 +326,7 @@ public static (string Name, string Content) CreateCSharpDelegateInit(
 
         public static string GetUniqueNameForType(CSharpType type)
         {
-            string name = type.Symbol.Name;
+            string name = type.Name;
             string genericTypeHash = "";
             INamedTypeSymbol? named = type.Symbol as INamedTypeSymbol;
             if (named != null && named.IsGenericType)