Fix refcnt errors (split from #958) (#1001)

amos402 · filmor · commit 5f56ebc8f078 · 2019-12-02T13:31:05.000+01:00
* Add exception helper
* Fixed refcnt error in ExtensionType.FinalizeObject
* Fixed typename leaking
* Fix refcnt error by using `using`
diff --git a/.editorconfig b/.editorconfig
@@ -25,7 +25,7 @@ dotnet_sort_system_directives_first = true
 dotnet_separate_import_directive_groups = true
 
 [*.cs]
-csharp_new_line_before_open_brace = true
+csharp_new_line_before_open_brace = all
 csharp_new_line_before_else = true
 csharp_new_line_before_catch = true
 csharp_new_line_before_finally = true
diff --git a/src/runtime/extensiontype.cs b/src/runtime/extensiontype.cs
@@ -38,6 +38,7 @@ public ExtensionType()
 
             Runtime.PyObject_GC_UnTrack(py);
 
+            // Steals a ref to tpHandle.
             tpHandle = tp;
             pyHandle = py;
             gcHandle = gc;
@@ -50,7 +51,7 @@ public ExtensionType()
         public static void FinalizeObject(ManagedType self)
         {
             Runtime.PyObject_GC_Del(self.pyHandle);
-            Runtime.XDecref(self.tpHandle);
+            // Not necessary for decref of `tpHandle`.
             self.gcHandle.Free();
         }
 
diff --git a/src/runtime/metatype.cs b/src/runtime/metatype.cs
@@ -266,6 +266,7 @@ private static IntPtr DoInstanceCheck(IntPtr tp, IntPtr args, bool checkType)
                 return Runtime.PyFalse;
             }
 
+            Runtime.XIncref(args);
             using (var argsObj = new PyList(args))
             {
                 if (argsObj.Length() != 1)
diff --git a/src/runtime/pythonexception.cs b/src/runtime/pythonexception.cs
@@ -1,4 +1,5 @@
 using System;
+using System.Runtime.CompilerServices;
 
 namespace Python.Runtime
 {
@@ -190,5 +191,21 @@ public static bool Matches(IntPtr ob)
         {
             return Runtime.PyErr_ExceptionMatches(ob) != 0;
         }
+
+        public static void ThrowIfIsNull(IntPtr ob)
+        {
+            if (ob == IntPtr.Zero)
+            {
+                throw new PythonException();
+            }
+        }
+
+        public static void ThrowIfIsNotZero(int value)
+        {
+            if (value != 0)
+            {
+                throw new PythonException();
+            }
+        }
     }
 }
diff --git a/src/runtime/runtime.cs b/src/runtime/runtime.cs
@@ -1368,6 +1368,10 @@ internal static IntPtr PyUnicode_FromStringAndSize(IntPtr value, long size)
 
         [DllImport(_PythonDll, CallingConvention = CallingConvention.Cdecl)]
         private static extern IntPtr PyUnicode_FromStringAndSize(IntPtr value, IntPtr size);
+
+        [DllImport(_PythonDll, CallingConvention = CallingConvention.Cdecl)]
+        internal static extern IntPtr PyUnicode_AsUTF8(IntPtr unicode);
+
 #elif PYTHON2
         internal static IntPtr PyString_FromStringAndSize(string value, long size)
         {
diff --git a/src/runtime/typemanager.cs b/src/runtime/typemanager.cs
@@ -409,12 +409,8 @@ internal static IntPtr AllocateTypeObject(string name)
             // the Python version of the type name - otherwise we'd have to
             // allocate the tp_name and would have no way to free it.
 #if PYTHON3
-            // For python3 we leak two objects. One for the ASCII representation
-            // required for tp_name, and another for the Unicode representation
-            // for ht_name.
-            IntPtr temp = Runtime.PyBytes_FromString(name);
-            IntPtr raw = Runtime.PyBytes_AS_STRING(temp);
-            temp = Runtime.PyUnicode_FromString(name);
+            IntPtr temp = Runtime.PyUnicode_FromString(name);
+            IntPtr raw = Runtime.PyUnicode_AsUTF8(temp);
 #elif PYTHON2
             IntPtr temp = Runtime.PyString_FromString(name);
             IntPtr raw = Runtime.PyString_AsString(temp);

Original file line number	Diff line number	Diff line change
`@@ -38,6 +38,7 @@ public ExtensionType()`
`38`	`38`
`39`	`39`	`Runtime.PyObject_GC_UnTrack(py);`
`40`	`40`
	`41`	`+ // Steals a ref to tpHandle.`
`41`	`42`	`tpHandle = tp;`
`42`	`43`	`pyHandle = py;`
`43`	`44`	`gcHandle = gc;`
`@@ -50,7 +51,7 @@ public ExtensionType()`
`50`	`51`	`public static void FinalizeObject(ManagedType self)`
`51`	`52`	`{`
`52`	`53`	`Runtime.PyObject_GC_Del(self.pyHandle);`
`53`		`- Runtime.XDecref(self.tpHandle);`
	`54`	+ // Not necessary for decref of `tpHandle`.
`54`	`55`	`self.gcHandle.Free();`
`55`	`56`	`}`
`56`	`57`
Original file line number	Diff line number	Diff line change
`@@ -266,6 +266,7 @@ private static IntPtr DoInstanceCheck(IntPtr tp, IntPtr args, bool checkType)`
`266`	`266`	`return Runtime.PyFalse;`
`267`	`267`	`}`
`268`	`268`
	`269`	`+ Runtime.XIncref(args);`
`269`	`270`	`using (var argsObj = new PyList(args))`
`270`	`271`	`{`
`271`	`272`	`if (argsObj.Length() != 1)`
Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,5 @@`
`1`	`1`	`using System;`
	`2`	`+using System.Runtime.CompilerServices;`
`2`	`3`
`3`	`4`	`namespace Python.Runtime`
`4`	`5`	`{`
`@@ -190,5 +191,21 @@ public static bool Matches(IntPtr ob)`
`190`	`191`	`{`
`191`	`192`	`return Runtime.PyErr_ExceptionMatches(ob) != 0;`
`192`	`193`	`}`
	`194`	`+`
	`195`	`+ public static void ThrowIfIsNull(IntPtr ob)`
	`196`	`+ {`
	`197`	`+ if (ob == IntPtr.Zero)`
	`198`	`+ {`
	`199`	`+ throw new PythonException();`
	`200`	`+ }`
	`201`	`+ }`
	`202`	`+`
	`203`	`+ public static void ThrowIfIsNotZero(int value)`
	`204`	`+ {`
	`205`	`+ if (value != 0)`
	`206`	`+ {`
	`207`	`+ throw new PythonException();`
	`208`	`+ }`
	`209`	`+ }`
`193`	`210`	`}`
`194`	`211`	`}`