Skip to content

Commit 7e73b0d

Browse files
filmorfilmor
authored
Merge pull request #1330 from losttech/bugs/1327
Fixed segfault in ClassDerived.tp_dealloc
2 parents 44a36dc + 039b3ca commit 7e73b0d

File tree

6 files changed

+45
-42
lines changed

6 files changed

+45
-42
lines changed

‎src/runtime/classderived.cs

+2-1
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,8 @@ internal ClassDerivedObject(Type tp) : base(tp)
7575
// So we don't call PyObject_GC_Del here and instead we set the python
7676
// reference to a weak reference so that the C# object can be collected.
7777
GCHandle gc = GCHandle.Alloc(self, GCHandleType.Weak);
78-
Marshal.WriteIntPtr(self.pyHandle, ObjectOffset.magic(self.tpHandle), (IntPtr)gc);
78+
int gcOffset = ObjectOffset.magic(Runtime.PyObject_TYPE(self.pyHandle));
79+
Marshal.WriteIntPtr(self.pyHandle, gcOffset, (IntPtr)gc);
7980
self.gcHandle.Free();
8081
self.gcHandle = gc;
8182
}

‎src/runtime/clrobject.cs

+2
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
using System;
2+
using System.Diagnostics;
23
using System.Runtime.InteropServices;
34

45
namespace Python.Runtime
@@ -85,6 +86,7 @@ internal static CLRObject Restore(object ob, IntPtr pyHandle, InterDomainContext
8586
pyHandle = pyHandle,
8687
tpHandle = Runtime.PyObject_TYPE(pyHandle)
8788
};
89+
Debug.Assert(co.tpHandle != IntPtr.Zero);
8890
co.Load(context);
8991
return co;
9092
}

‎src/runtime/converter.cs

+3-1
Original file line numberDiff line numberDiff line change
@@ -313,7 +313,9 @@ internal static bool ToManaged(IntPtr value, Type type,
313313
return Converter.ToManagedValue(value, type, out result, setError);
314314
}
315315

316-
316+
internal static bool ToManagedValue(BorrowedReference value, Type obType,
317+
out object result, bool setError)
318+
=> ToManagedValue(value.DangerousGetAddress(), obType, out result, setError);
317319
internal static bool ToManagedValue(IntPtr value, Type obType,
318320
out object result, bool setError)
319321
{

‎src/runtime/native/TypeOffset.cs

+5-3
Original file line numberDiff line numberDiff line change
@@ -85,9 +85,11 @@ internal static void Use(ITypeOffsets offsets)
8585
internal static Dictionary<string, int> GetOffsets()
8686
{
8787
var properties = typeof(TypeOffset).GetProperties(FieldFlags);
88-
return properties.ToDictionary(
89-
keySelector: p => p.Name,
90-
elementSelector: p => (int)p.GetValue(obj: null, index: null));
88+
var result = properties.ToDictionary(
89+
keySelector: p => p.Name,
90+
elementSelector: p => (int)p.GetValue(obj: null, index: null));
91+
Debug.Assert(result.Values.Any(v => v != 0));
92+
return result;
9193
}
9294

9395
internal static int GetOffsetUncached(string name)

‎src/runtime/runtime.cs

+2
Original file line numberDiff line numberDiff line change
@@ -1641,6 +1641,8 @@ internal static bool PyDict_Check(IntPtr ob)
16411641
/// </summary>
16421642
[DllImport(_PythonDll, CallingConvention = CallingConvention.Cdecl)]
16431643
internal static extern IntPtr PyDict_GetItem(IntPtr pointer, IntPtr key);
1644+
[DllImport(_PythonDll, CallingConvention = CallingConvention.Cdecl)]
1645+
internal static extern BorrowedReference PyDict_GetItemWithError(BorrowedReference pointer, BorrowedReference key);
16441646

16451647
/// <summary>
16461648
/// Return value: Borrowed reference.

‎src/runtime/typemanager.cs

+31-37
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
using System.Runtime.InteropServices;
77
using System.Diagnostics;
88
using Python.Runtime.Slots;
9+
using static Python.Runtime.PythonException;
910

1011
namespace Python.Runtime
1112
{
@@ -142,7 +143,7 @@ internal static IntPtr GetTypeHandle(ManagedType obj, Type type)
142143
/// </summary>
143144
internal static IntPtr CreateType(Type impl)
144145
{
145-
IntPtr type = AllocateTypeObject(impl.Name);
146+
IntPtr type = AllocateTypeObject(impl.Name, metatype: Runtime.PyTypeType);
146147
int ob_size = ObjectOffset.Size(type);
147148

148149
// Set tp_basicsize to the size of our managed instance objects.
@@ -211,7 +212,7 @@ internal static IntPtr CreateType(ManagedType impl, Type clrType)
211212
base_ = bc.pyHandle;
212213
}
213214

214-
IntPtr type = AllocateTypeObject(name);
215+
IntPtr type = AllocateTypeObject(name, Runtime.PyCLRMetaType);
215216

216217
Marshal.WriteIntPtr(type, TypeOffset.ob_type, Runtime.PyCLRMetaType);
217218
Runtime.XIncref(Runtime.PyCLRMetaType);
@@ -302,6 +303,7 @@ internal static IntPtr CreateType(ManagedType impl, Type clrType)
302303

303304
internal static IntPtr CreateSubType(IntPtr py_name, IntPtr py_base_type, IntPtr py_dict)
304305
{
306+
var dictRef = new BorrowedReference(py_dict);
305307
// Utility to create a subtype of a managed type with the ability for the
306308
// a python subtype able to override the managed implementation
307309
string name = Runtime.GetManagedString(py_name);
@@ -311,40 +313,29 @@ internal static IntPtr CreateSubType(IntPtr py_name, IntPtr py_base_type, IntPtr
311313
object assembly = null;
312314
object namespaceStr = null;
313315

314-
var disposeList = new List<PyObject>();
315-
try
316+
using (var assemblyKey = new PyString("__assembly__"))
316317
{
317-
var assemblyKey = new PyObject(Converter.ToPython("__assembly__", typeof(string)));
318-
disposeList.Add(assemblyKey);
319-
if (0 != Runtime.PyMapping_HasKey(py_dict, assemblyKey.Handle))
318+
var assemblyPtr = Runtime.PyDict_GetItemWithError(dictRef, assemblyKey.Reference);
319+
if (assemblyPtr.IsNull)
320320
{
321-
var pyAssembly = new PyObject(Runtime.PyDict_GetItem(py_dict, assemblyKey.Handle));
322-
Runtime.XIncref(pyAssembly.Handle);
323-
disposeList.Add(pyAssembly);
324-
if (!Converter.ToManagedValue(pyAssembly.Handle, typeof(string), out assembly, false))
325-
{
326-
throw new InvalidCastException("Couldn't convert __assembly__ value to string");
327-
}
321+
if (Exceptions.ErrorOccurred()) return IntPtr.Zero;
322+
}
323+
else if (!Converter.ToManagedValue(assemblyPtr, typeof(string), out assembly, false))
324+
{
325+
return Exceptions.RaiseTypeError("Couldn't convert __assembly__ value to string");
328326
}
329327

330-
var namespaceKey = new PyObject(Converter.ToPythonImplicit("__namespace__"));
331-
disposeList.Add(namespaceKey);
332-
if (0 != Runtime.PyMapping_HasKey(py_dict, namespaceKey.Handle))
328+
using (var namespaceKey = new PyString("__namespace__"))
333329
{
334-
var pyNamespace = new PyObject(Runtime.PyDict_GetItem(py_dict, namespaceKey.Handle));
335-
Runtime.XIncref(pyNamespace.Handle);
336-
disposeList.Add(pyNamespace);
337-
if (!Converter.ToManagedValue(pyNamespace.Handle, typeof(string), out namespaceStr, false))
330+
var pyNamespace = Runtime.PyDict_GetItemWithError(dictRef, namespaceKey.Reference);
331+
if (pyNamespace.IsNull)
338332
{
339-
throw new InvalidCastException("Couldn't convert __namespace__ value to string");
333+
if (Exceptions.ErrorOccurred()) return IntPtr.Zero;
334+
}
335+
else if (!Converter.ToManagedValue(pyNamespace, typeof(string), out namespaceStr, false))
336+
{
337+
return Exceptions.RaiseTypeError("Couldn't convert __namespace__ value to string");
340338
}
341-
}
342-
}
343-
finally
344-
{
345-
foreach (PyObject o in disposeList)
346-
{
347-
o.Dispose();
348339
}
349340
}
350341

@@ -370,14 +361,14 @@ internal static IntPtr CreateSubType(IntPtr py_name, IntPtr py_base_type, IntPtr
370361
// by default the class dict will have all the C# methods in it, but as this is a
371362
// derived class we want the python overrides in there instead if they exist.
372363
IntPtr cls_dict = Marshal.ReadIntPtr(py_type, TypeOffset.tp_dict);
373-
Runtime.PyDict_Update(cls_dict, py_dict);
364+
ThrowIfIsNotZero(Runtime.PyDict_Update(cls_dict, py_dict));
374365
Runtime.XIncref(py_type);
375366
// Update the __classcell__ if it exists
376367
var cell = new BorrowedReference(Runtime.PyDict_GetItemString(cls_dict, "__classcell__"));
377368
if (!cell.IsNull)
378369
{
379-
Runtime.PyCell_Set(cell, py_type);
380-
Runtime.PyDict_DelItemString(cls_dict, "__classcell__");
370+
ThrowIfIsNotZero(Runtime.PyCell_Set(cell, py_type));
371+
ThrowIfIsNotZero(Runtime.PyDict_DelItemString(cls_dict, "__classcell__"));
381372
}
382373

383374
return py_type;
@@ -436,12 +427,15 @@ internal static IntPtr CreateMetaType(Type impl, out SlotsHolder slotsHolder)
436427
// the standard type slots, and has to subclass PyType_Type for
437428
// certain functions in the C runtime to work correctly with it.
438429

439-
IntPtr type = AllocateTypeObject("CLR Metatype");
440-
IntPtr py_type = Runtime.PyTypeType;
430+
IntPtr type = AllocateTypeObject("CLR Metatype", metatype: Runtime.PyTypeType);
441431

432+
IntPtr py_type = Runtime.PyTypeType;
442433
Marshal.WriteIntPtr(type, TypeOffset.tp_base, py_type);
443434
Runtime.XIncref(py_type);
444435

436+
int size = TypeOffset.magic() + IntPtr.Size;
437+
Marshal.WriteIntPtr(type, TypeOffset.tp_basicsize, new IntPtr(size));
438+
445439
const int flags = TypeFlags.Default
446440
| TypeFlags.Managed
447441
| TypeFlags.HeapType
@@ -531,7 +525,7 @@ internal static IntPtr BasicSubType(string name, IntPtr base_, Type impl)
531525
// Utility to create a subtype of a std Python type, but with
532526
// a managed type able to override implementation
533527

534-
IntPtr type = AllocateTypeObject(name);
528+
IntPtr type = AllocateTypeObject(name, metatype: Runtime.PyTypeType);
535529
//Marshal.WriteIntPtr(type, TypeOffset.tp_basicsize, (IntPtr)obSize);
536530
//Marshal.WriteIntPtr(type, TypeOffset.tp_itemsize, IntPtr.Zero);
537531

@@ -573,9 +567,9 @@ internal static IntPtr BasicSubType(string name, IntPtr base_, Type impl)
573567
/// <summary>
574568
/// Utility method to allocate a type object &amp; do basic initialization.
575569
/// </summary>
576-
internal static IntPtr AllocateTypeObject(string name)
570+
internal static IntPtr AllocateTypeObject(string name, IntPtr metatype)
577571
{
578-
IntPtr type = Runtime.PyType_GenericAlloc(Runtime.PyTypeType, 0);
572+
IntPtr type = Runtime.PyType_GenericAlloc(metatype, 0);
579573
// Clr type would not use __slots__,
580574
// and the PyMemberDef after PyHeapTypeObject will have other uses(e.g. type handle),
581575
// thus set the ob_size to 0 for avoiding slots iterations.

0 commit comments

Comments
 (0)