Merge pull request #526 from gircore/feature/tests_enforce_garbage_co…

…llection Do not free unowned record handles
gircore · Jan 19, 2022 · 187f691 · 187f691
2 parents c138224 + a2cd5b0
commit 187f691
Show file tree

Hide file tree

Showing 22 changed files with 260 additions and 59 deletions.
diff --git a/src/Generation/Generator3/Converter/Model/ParameterConverter.ToManaged.cs b/src/Generation/Generator3/Converter/Model/ParameterConverter.ToManaged.cs
@@ -110,7 +110,12 @@ public static partial class ParameterConverter
                 var ownedHandle = from.Transfer == Transfer.Full;
 
                 variableName = from.GetConvertedName();
-                return $"var {variableName} = new {record.GetFullyQualifiedPublicClassName()}(new {record.GetFullyQualifiedInternalHandle()}({from.GetPublicName()}, {ownedHandle.ToString().ToLower()}));";
+
+                var handleClass = ownedHandle
+                    ? record.GetFullyQualifiedInternalOwnedHandle()
+                    : record.GetFullyQualifiedInternalUnownedHandle();
+
+                return $"var {variableName} = new {record.GetFullyQualifiedPublicClassName()}(new {handleClass}({from.GetPublicName()}));";
             }
 
             #endregion

diff --git a/src/Generation/Generator3/Converter/Model/ParameterConverter.ToNative.cs b/src/Generation/Generator3/Converter/Model/ParameterConverter.ToNative.cs
@@ -150,7 +150,7 @@ public static partial class ParameterConverter
                 if (from.Nullable)
                 {
                     var record = (GirModel.Record) from.AnyType.AsT0;
-                    variableName = from.GetPublicName() + "?.Handle ?? " + record.GetFullyQualifiedInternalNullHandle();
+                    variableName = from.GetPublicName() + "?.Handle ?? " + record.GetFullyQualifiedInternalNullHandleInstance();
                 }
                 else
                 {

diff --git a/src/Generation/Generator3/Converter/Name/RecordNameConverter.cs b/src/Generation/Generator3/Converter/Name/RecordNameConverter.cs
@@ -8,8 +8,14 @@ public static string GetFullyQualifiedInternalStructName(this GirModel.Record re
         public static string GetFullyQualifiedInternalHandle(this GirModel.Record record)
             => record.Namespace.GetInternalName() + "." + GetInternalHandleName(record);
 
-        public static string GetFullyQualifiedInternalNullHandle(this GirModel.Record record)
-            => GetFullyQualifiedInternalHandle(record) + ".Null";
+        public static string GetFullyQualifiedInternalNullHandleInstance(this GirModel.Record record)
+            => record.Namespace.GetInternalName() + "." + GetInternalNullHandleName(record) + "." + "Instance";
+
+        public static string GetFullyQualifiedInternalOwnedHandle(this GirModel.Record record)
+            => record.Namespace.GetInternalName() + "." + GetInternalOwnedHandleName(record);
+
+        public static string GetFullyQualifiedInternalUnownedHandle(this GirModel.Record record)
+            => record.Namespace.GetInternalName() + "." + GetInternalUnownedHandleName(record);
 
         public static string GetFullyQualifiedInternalManagedHandleCreateMethod(this GirModel.Record record)
             => record.Namespace.GetInternalName() + "." + GetInternalManagedHandleName(record) + ".Create";
@@ -26,6 +32,15 @@ public static string GetInternalStructName(this GirModel.Record record)
         public static string GetInternalHandleName(this GirModel.Record record)
             => record.Name + "Handle";
 
+        public static string GetInternalNullHandleName(this GirModel.Record record)
+            => record.Name + "NullHandle";
+
+        public static string GetInternalOwnedHandleName(this GirModel.Record record)
+            => record.Name + "OwnedHandle";
+
+        public static string GetInternalUnownedHandleName(this GirModel.Record record)
+            => record.Name + "UnownedHandle";
+
         public static string GetInternalManagedHandleName(this GirModel.Record record)
             => record.Name + "ManagedHandle";
     }

diff --git a/src/Generation/Generator3/Generation/Callback/InternalDelegate/InternalDelegateModel.cs b/src/Generation/Generator3/Generation/Callback/InternalDelegate/InternalDelegateModel.cs
@@ -13,7 +13,7 @@ public class InternalDelegateModel
         public string Name => _callback.Name;
         public string NamespaceName => _callback.Namespace.GetInternalName();
 
-        public ReturnType ReturnType => _returnType ??= _callback.ReturnType.CreateInternalModel();
+        public ReturnType ReturnType => _returnType ??= _callback.ReturnType.CreateInternalModelForCallback();
         public IEnumerable<Parameter> Parameters => _parameters ??= _callback.Parameters.CreateInternalModelsForCallback();
 
         public InternalDelegateModel(GirModel.Callback callback)

diff --git a/src/Generation/Generator3/Generation/Record/InternalHandle/FreeMemoryCallRenderer.cs b/src/Generation/Generator3/Generation/Record/InternalHandle/FreeMemoryCallRenderer.cs
@@ -5,7 +5,7 @@ public static class FreeMemoryCallRenderer
         public static string RenderFreeCall(this InternalHandleModel model)
         {
             return model.FreeMethod is null
-                ? $"throw new System.Exception(\"Can't free native handle of type \\\"{model.InternalNamespaceName}.{model.Name}\\\".\");"
+                ? $"throw new System.Exception(\"Can't free native handle of type \\\"{model.InternalNamespaceName}.{model.OwnedHandleName}\\\".\");"
                 : @$"{model.FreeMethod.Name}(handle);
 return true;";
         }

diff --git a/src/Generation/Generator3/Generation/Record/InternalHandle/InternalHandleGenerator.cs b/src/Generation/Generator3/Generation/Record/InternalHandle/InternalHandleGenerator.cs
@@ -19,7 +19,7 @@ public void Generate(GirModel.Record record)
             {
                 var model = new InternalHandleModel(record);
                 var source = _template.Render(model);
-                var codeUnit = new CodeUnit(record.Namespace.GetCanonicalName(), model.Name, source);
+                var codeUnit = new CodeUnit(record.Namespace.GetCanonicalName(), model.HandleName, source);
                 _publisher.Publish(codeUnit);
             }
             catch

diff --git a/src/Generation/Generator3/Generation/Record/InternalHandle/InternalHandleModel.cs b/src/Generation/Generator3/Generation/Record/InternalHandle/InternalHandleModel.cs
@@ -6,7 +6,10 @@ namespace Generator3.Generation.Record
 {
     public class InternalHandleModel
     {
-        public string Name => Record.GetInternalHandleName();
+        public string HandleName => Record.GetInternalHandleName();
+        public string NullHandleName => Record.GetInternalNullHandleName();
+        public string OwnedHandleName => Record.GetInternalOwnedHandleName();
+        public string UnownedHandleName => Record.GetInternalUnownedHandleName();
         public string InternalNamespaceName => Record.Namespace.GetInternalName();
         public string NamespaceName => Record.Namespace.Name;
         public GirModel.Record Record { get; }

diff --git a/src/Generation/Generator3/Generation/Record/InternalHandle/InternalHandleTemplate.cs b/src/Generation/Generator3/Generation/Record/InternalHandle/InternalHandleTemplate.cs
@@ -14,17 +14,33 @@ namespace { model.InternalNamespaceName }
 {{
     // AUTOGENERATED FILE - DO NOT MODIFY
 
-    public partial class { model.Name } : SafeHandle
+    public abstract class {model.HandleName} : SafeHandle
     {{
-        public static {model.Name} Null = new {model.Name}();
+        protected {model.HandleName}(bool ownsHandle) : base(IntPtr.Zero, ownsHandle) {{ }}
 
-        protected {model.Name}() : base(IntPtr.Zero, true) {{}}
-        public {model.Name}(IntPtr handle, bool ownsHandle) : base(IntPtr.Zero, ownsHandle)
+        public sealed override bool IsInvalid => handle == IntPtr.Zero;
+    }}
+
+    public class {model.NullHandleName} : {model.HandleName}
+    {{
+        public static {model.NullHandleName} Instance = new {model.NullHandleName}();
+    
+        private {model.NullHandleName}() : base(true) {{ }}
+
+        protected override bool ReleaseHandle()
         {{
-            SetHandle(handle);
+            throw new System.Exception(""It is not allowed to free a \""{model.InternalNamespaceName}.{model.NullHandleName}\""."");
         }}
+    }}
 
-        public sealed override bool IsInvalid => handle == IntPtr.Zero;
+    public partial class {model.OwnedHandleName} : {model.HandleName}
+    {{
+        private {model.OwnedHandleName}() : base(true) {{ }}
+
+        public {model.OwnedHandleName}(IntPtr handle) : base(true)
+        {{
+            SetHandle(handle);
+        }}
 
         protected override bool ReleaseHandle()
         {{
@@ -33,6 +49,21 @@ protected override bool ReleaseHandle()
 
         {model.RenderFreeFunction()}
     }}
+
+    public partial class {model.UnownedHandleName} : {model.HandleName}
+    {{
+        private {model.UnownedHandleName}() : base(false) {{ }}
+
+        public {model.UnownedHandleName}(IntPtr handle) : base(false)
+        {{
+            SetHandle(handle);
+        }}    
+
+        protected override bool ReleaseHandle()
+        {{
+            throw new System.Exception(""It is not allowed to free a \""{model.InternalNamespaceName}.{model.UnownedHandleName}\""."");
+        }}
+    }}
 }}";
         }
     }

diff --git a/...ation/Generator3/Generation/Record/InternalManagedHandle/InternalManagedHandleTemplate.cs b/...ation/Generator3/Generation/Record/InternalManagedHandle/InternalManagedHandleTemplate.cs
@@ -14,7 +14,7 @@ namespace { model.NamespaceName }
 {{
     // AUTOGENERATED FILE - DO NOT MODIFY
 
-    public partial class { model.Name } : {model.BaseHandle}
+    public class { model.Name } : {model.BaseHandle}
     {{
         public static {model.BaseHandle} Create({model.InternalStruct}? data = null)
         {{
@@ -34,7 +34,10 @@ public partial class { model.Name } : {model.BaseHandle}
             return new {model.Name}(ptr);
         }}
             
-        private {model.Name}(IntPtr handle) : base(handle, true) {{ }}
+        private {model.Name}(IntPtr handle) : base(true) 
+        {{
+            SetHandle(handle); 
+        }}
     
         protected override bool ReleaseHandle()
         {{

diff --git a/src/Generation/Generator3/Generation/Record/PublicClass/PublicClassTemplate.cs b/src/Generation/Generator3/Generation/Record/PublicClass/PublicClassTemplate.cs
@@ -24,15 +24,13 @@ public partial class {model.Name} : GLib.IHandle
 
         // Override this to perform additional steps in the constructor
         partial void Initialize();
-
-        public {model.Name}(IntPtr ptr, bool ownsHandle) : this(new Internal.{model.InternalHandleName}(ptr, ownsHandle)){{ }}
         
         public {model.Name}(Internal.{model.InternalHandleName} handle)
         {{
             _handle = handle;
             Initialize();
         }}
-        
+
         // TODO: Default Constructor (allocate in managed memory and free on Dispose?)
         // We need to be able to create instances of records with full access to
         // fields, e.g. Gdk.Rectangle, Gtk.TreeIter, etc. 

diff --git a/src/Generation/Generator3/Model/Internal/Parameter/RecordParameter.cs b/src/Generation/Generator3/Model/Internal/Parameter/RecordParameter.cs
@@ -1,4 +1,6 @@
-using Generator3.Converter;
+using System;
+using Generator3.Converter;
+using GirModel;
 
 namespace Generator3.Model.Internal
 {
@@ -7,7 +9,18 @@ public class RecordParameter : Parameter
         private GirModel.Record Type => (GirModel.Record) Model.AnyType.AsT0;
 
         //Native records are represented as SafeHandles and are not nullable
-        public override string NullableTypeName => Type.GetFullyQualifiedInternalHandle();
+        public override string NullableTypeName => Model switch
+        {
+            { Direction: global::GirModel.Direction.In } => Type.GetFullyQualifiedInternalHandle(),
+            { CallerAllocates: true } => Type.GetFullyQualifiedInternalHandle(),
+            { CallerAllocates: false, Direction: global::GirModel.Direction.InOut, Transfer: Transfer.Full } => Type.GetFullyQualifiedInternalOwnedHandle(),
+            { CallerAllocates: false, Direction: global::GirModel.Direction.InOut, Transfer: Transfer.Container } => Type.GetFullyQualifiedInternalOwnedHandle(),
+            { CallerAllocates: false, Direction: global::GirModel.Direction.InOut, Transfer: Transfer.None } => Type.GetFullyQualifiedInternalUnownedHandle(),
+            { CallerAllocates: false, Direction: global::GirModel.Direction.Out, Transfer: Transfer.Full } => Type.GetFullyQualifiedInternalOwnedHandle(),
+            { CallerAllocates: false, Direction: global::GirModel.Direction.Out, Transfer: Transfer.Container } => Type.GetFullyQualifiedInternalOwnedHandle(),
+            { CallerAllocates: false, Direction: global::GirModel.Direction.Out, Transfer: Transfer.None } => Type.GetFullyQualifiedInternalUnownedHandle(),
+            _ => throw new Exception($"Can't detect parameter type: CallerAllocates={Model.CallerAllocates} Direction={Model.Direction} Transfer={Model.Transfer}")
+        };
 
         public override string Direction => Model.GetDirection(
             @in: ParameterDirection.In,

diff --git a/src/Generation/Generator3/Model/Internal/ReturnType/CallbackReturnTypeFactory.cs b/src/Generation/Generator3/Model/Internal/ReturnType/CallbackReturnTypeFactory.cs
@@ -0,0 +1,33 @@
+namespace Generator3.Model.Internal
+{
+    public static class CallbackReturnTypeFactory
+    {
+        public static ReturnType CreateInternalModelForCallback(this GirModel.ReturnType returnValue) => returnValue.AnyType.Match<ReturnType>(
+            type => type switch
+            {
+                GirModel.PrimitiveValueType => new PrimitiveValueReturnType(returnValue),
+                GirModel.Bitfield => new BitfieldReturnType(returnValue),
+                GirModel.Enumeration => new EnumerationReturnType(returnValue),
+                GirModel.Utf8String => new Utf8StringReturnType(returnValue),
+                GirModel.PlatformString => new PlatformStringReturnType(returnValue),
+                GirModel.Record => new RecordReturnTypeForCallback(returnValue),
+                GirModel.Union => new UnionReturnType(returnValue),
+                GirModel.Class => new ClassReturnType(returnValue),
+                GirModel.Interface => new InterfaceReturnType(returnValue),
+                GirModel.Pointer => new PointerReturnType(returnValue),
+                _ => new StandardReturnType(returnValue)
+            },
+            arrayType => arrayType.AnyType.Match<ReturnType>(
+                type => type switch
+                {
+                    GirModel.String => new ArrayStringReturnType(returnValue),
+                    GirModel.Record => new ArrayRecordReturnType(returnValue),
+                    GirModel.Class => new ArrayClassReturnType(returnValue),
+                    GirModel.PrimitiveValueType => new ArrayPrimitiveValueReturnType(returnValue),
+                    _ => new StandardReturnType(returnValue)
+                },
+                _ => new StandardReturnType(returnValue)
+            )
+        );
+    }
+}
diff --git a/src/Generation/Generator3/Model/Internal/ReturnType/RecordReturnType.cs b/src/Generation/Generator3/Model/Internal/ReturnType/RecordReturnType.cs
@@ -1,14 +1,17 @@
 using Generator3.Converter;
+using GirModel;
 
 namespace Generator3.Model.Internal
 {
     public class RecordReturnType : ReturnType
     {
         private GirModel.Record Type => (GirModel.Record) Model.AnyType.AsT0;
 
-        public override string NullableTypeName => IsPointer
-            ? Type.GetFullyQualifiedInternalHandle()
-            : Type.GetFullyQualifiedInternalStructName();
+        public override string NullableTypeName => !IsPointer
+            ? Type.GetFullyQualifiedInternalStructName()
+            : Model.Transfer == Transfer.None
+                ? Type.GetFullyQualifiedInternalUnownedHandle()
+                : Type.GetFullyQualifiedInternalOwnedHandle();
 
         protected internal RecordReturnType(GirModel.ReturnType returnValue) : base(returnValue)
         {

diff --git a/src/Generation/Generator3/Model/Internal/ReturnType/RecordReturnTypeForCallback.cs b/src/Generation/Generator3/Model/Internal/ReturnType/RecordReturnTypeForCallback.cs
@@ -0,0 +1,19 @@
+using Generator3.Converter;
+using GirModel;
+
+namespace Generator3.Model.Internal
+{
+    public class RecordReturnTypeForCallback : ReturnType
+    {
+        private GirModel.Record Type => (GirModel.Record) Model.AnyType.AsT0;
+
+        public override string NullableTypeName => !IsPointer
+            ? Type.GetFullyQualifiedInternalStructName()
+            : Type.GetFullyQualifiedInternalHandle();
+
+        protected internal RecordReturnTypeForCallback(GirModel.ReturnType returnValue) : base(returnValue)
+        {
+            returnValue.AnyType.VerifyType<GirModel.Record>();
+        }
+    }
+}
diff --git a/src/GirCore.Testing.props b/src/GirCore.Testing.props
@@ -5,10 +5,10 @@
   </PropertyGroup>
 
   <ItemGroup>
-    <PackageReference Include="FluentAssertions" Version="6.1.0" />
-    <PackageReference Include="Microsoft.NET.Test.Sdk" Version="16.11.0" />
-    <PackageReference Include="MSTest.TestAdapter" Version="2.2.6" />
-    <PackageReference Include="MSTest.TestFramework" Version="2.2.6" />
+    <PackageReference Include="FluentAssertions" Version="6.3.0" />
+    <PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.0.0" />
+    <PackageReference Include="MSTest.TestAdapter" Version="2.2.8" />
+    <PackageReference Include="MSTest.TestFramework" Version="2.2.8" />
     <PackageReference Include="coverlet.collector" Version="3.1.0" />
     <PackageReference Include="Moq" Version="4.16.1" />
   </ItemGroup>

diff --git a/src/Libs/GLib-2.0/Records/MainLoop.cs b/src/Libs/GLib-2.0/Records/MainLoop.cs
@@ -5,7 +5,7 @@ public sealed partial class MainLoop
         public MainLoop(MainContext context, bool isRunning = false)
             : this(context.Handle, isRunning) { }
 
-        public MainLoop() : this(Internal.MainContextHandle.Null, false) { }
+        public MainLoop() : this(Internal.MainContextNullHandle.Instance, false) { }
 
         private MainLoop(Internal.MainContextHandle context, bool isRunning)
         {

diff --git a/src/Libs/GLib-2.0/Records/Variant.cs b/src/Libs/GLib-2.0/Records/Variant.cs
@@ -94,6 +94,6 @@ public void Dispose()
     public static class VariantExtension
     {
         public static VariantHandle GetSafeHandle(this Variant? variant)
-            => variant is null ? VariantHandle.Null : variant.Handle;
+            => variant is null ? VariantNullHandle.Instance : variant.Handle;
     }
 }
diff --git a/src/Libs/GObject-2.0/Internal/Classes/ObjectWrapper.cs b/src/Libs/GObject-2.0/Internal/Classes/ObjectWrapper.cs
@@ -37,7 +37,7 @@ public static T WrapHandle<T>(IntPtr handle, bool ownedRef) where T : class, IHa
             Type gtype = GetTypeFromInstance(handle);
 
             Debug.Assert(
-                condition: Marshal.PtrToStringUTF8(Functions.TypeName(gtype.Value)) == Marshal.PtrToStringUTF8(Functions.TypeNameFromInstance(new TypeInstanceHandle(handle, false))),
+                condition: Marshal.PtrToStringUTF8(Functions.TypeName(gtype.Value)) == Marshal.PtrToStringUTF8(Functions.TypeNameFromInstance(new TypeInstanceUnownedHandle(handle))),
                 message: "GType name of instance and class do not match"
             );