Skip to content

Commit 150a900

Browse files
tonyrobertstonyroberts
committed
Make sure PyObject instances are disposed cleanly.
Waiting for the GC can cause problems as unreachable objects may be collected by Python.
1 parent ee15bf4 commit 150a900

File tree

2 files changed

+128
-137
lines changed

2 files changed

+128
-137
lines changed

‎src/runtime/classderived.cs

Lines changed: 120 additions & 131 deletions
Original file line numberDiff line numberDiff line change
@@ -164,18 +164,20 @@ internal static Type CreateDerivedType(string name,
164164
if (py_dict != IntPtr.Zero && Runtime.PyDict_Check(py_dict))
165165
{
166166
Runtime.Incref(py_dict);
167-
PyDict dict = new PyDict(py_dict);
168-
169-
foreach (PyObject pyKey in dict.Keys())
167+
using (PyDict dict = new PyDict(py_dict))
168+
using (PyObject keys = dict.Keys())
170169
{
171-
PyObject value = dict[pyKey];
172-
if (value.HasAttr("_clr_property_type_"))
170+
foreach (PyObject pyKey in keys)
173171
{
174-
string propertyName = pyKey.ToString();
175-
pyProperties.Add(propertyName);
172+
using (PyObject value = dict[pyKey])
173+
if (value.HasAttr("_clr_property_type_"))
174+
{
175+
string propertyName = pyKey.ToString();
176+
pyProperties.Add(propertyName);
176177

177-
// Add the property to the type
178-
AddPythonProperty(propertyName, value, typeBuilder);
178+
// Add the property to the type
179+
AddPythonProperty(propertyName, value, typeBuilder);
180+
}
179181
}
180182
}
181183
}
@@ -204,21 +206,23 @@ internal static Type CreateDerivedType(string name,
204206
if (py_dict != IntPtr.Zero && Runtime.PyDict_Check(py_dict))
205207
{
206208
Runtime.Incref(py_dict);
207-
PyDict dict = new PyDict(py_dict);
208-
209-
foreach (PyObject pyKey in dict.Keys())
209+
using (PyDict dict = new PyDict(py_dict))
210+
using (PyObject keys = dict.Keys())
210211
{
211-
PyObject value = dict[pyKey];
212-
if (value.HasAttr("_clr_return_type_") && value.HasAttr("_clr_arg_types_"))
212+
foreach (PyObject pyKey in keys)
213213
{
214-
string methodName = pyKey.ToString();
214+
using (PyObject value = dict[pyKey])
215+
if (value.HasAttr("_clr_return_type_") && value.HasAttr("_clr_arg_types_"))
216+
{
217+
string methodName = pyKey.ToString();
215218

216-
// if this method has already been redirected to the python method skip it
217-
if (virtualMethods.Contains(methodName))
218-
continue;
219+
// if this method has already been redirected to the python method skip it
220+
if (virtualMethods.Contains(methodName))
221+
continue;
219222

220-
// Add the method to the type
221-
AddPythonMethod(methodName, value, typeBuilder);
223+
// Add the method to the type
224+
AddPythonMethod(methodName, value, typeBuilder);
225+
}
222226
}
223227
}
224228
}
@@ -386,62 +390,64 @@ private static void AddPythonMethod(string methodName, PyObject func, TypeBuilde
386390
if (func.HasAttr("_clr_method_name_"))
387391
methodName = func.GetAttr("_clr_method_name_").ToString();
388392

389-
PyObject pyReturnType = func.GetAttr("_clr_return_type_");
390-
Type returnType = pyReturnType.AsManagedObject(typeof(Type)) as Type;
391-
if (returnType == null)
392-
returnType = typeof(void);
393-
394-
PyObject pyArgTypes = func.GetAttr("_clr_arg_types_");
395-
if (!pyArgTypes.IsIterable())
396-
throw new ArgumentException("_clr_arg_types_ must be a list or tuple of CLR types");
397-
398-
List<Type> argTypes = new List<Type>();
399-
foreach (PyObject pyArgType in pyArgTypes)
393+
using (PyObject pyReturnType = func.GetAttr("_clr_return_type_"))
394+
using (PyObject pyArgTypes = func.GetAttr("_clr_arg_types_"))
400395
{
401-
Type argType = pyArgType.AsManagedObject(typeof(Type)) as Type;
402-
if (argType == null)
396+
Type returnType = pyReturnType.AsManagedObject(typeof(Type)) as Type;
397+
if (returnType == null)
398+
returnType = typeof(void);
399+
400+
if (!pyArgTypes.IsIterable())
403401
throw new ArgumentException("_clr_arg_types_ must be a list or tuple of CLR types");
404-
argTypes.Add(argType);
405-
}
406402

407-
// add the method to call back into python
408-
MethodAttributes methodAttribs = MethodAttributes.Public |
409-
MethodAttributes.Virtual |
410-
MethodAttributes.ReuseSlot |
411-
MethodAttributes.HideBySig;
403+
List<Type> argTypes = new List<Type>();
404+
foreach (PyObject pyArgType in pyArgTypes)
405+
{
406+
Type argType = pyArgType.AsManagedObject(typeof(Type)) as Type;
407+
if (argType == null)
408+
throw new ArgumentException("_clr_arg_types_ must be a list or tuple of CLR types");
409+
argTypes.Add(argType);
410+
}
412411

413-
MethodBuilder methodBuilder = typeBuilder.DefineMethod(methodName,
414-
methodAttribs,
415-
returnType,
416-
argTypes.ToArray());
412+
// add the method to call back into python
413+
MethodAttributes methodAttribs = MethodAttributes.Public |
414+
MethodAttributes.Virtual |
415+
MethodAttributes.ReuseSlot |
416+
MethodAttributes.HideBySig;
417417

418-
ILGenerator il = methodBuilder.GetILGenerator();
419-
il.DeclareLocal(typeof(Object[]));
420-
il.Emit(OpCodes.Ldarg_0);
421-
il.Emit(OpCodes.Ldstr, methodName);
422-
il.Emit(OpCodes.Ldnull); // don't fall back to the base type's method
423-
il.Emit(OpCodes.Ldc_I4, argTypes.Count);
424-
il.Emit(OpCodes.Newarr, typeof(System.Object));
425-
il.Emit(OpCodes.Stloc_0);
426-
for (int i = 0; i < argTypes.Count; ++i)
427-
{
418+
MethodBuilder methodBuilder = typeBuilder.DefineMethod(methodName,
419+
methodAttribs,
420+
returnType,
421+
argTypes.ToArray());
422+
423+
ILGenerator il = methodBuilder.GetILGenerator();
424+
il.DeclareLocal(typeof(Object[]));
425+
il.Emit(OpCodes.Ldarg_0);
426+
il.Emit(OpCodes.Ldstr, methodName);
427+
il.Emit(OpCodes.Ldnull); // don't fall back to the base type's method
428+
il.Emit(OpCodes.Ldc_I4, argTypes.Count);
429+
il.Emit(OpCodes.Newarr, typeof(System.Object));
430+
il.Emit(OpCodes.Stloc_0);
431+
for (int i = 0; i < argTypes.Count; ++i)
432+
{
433+
il.Emit(OpCodes.Ldloc_0);
434+
il.Emit(OpCodes.Ldc_I4, i);
435+
il.Emit(OpCodes.Ldarg, i + 1);
436+
if (argTypes[i].IsValueType)
437+
il.Emit(OpCodes.Box, argTypes[i]);
438+
il.Emit(OpCodes.Stelem, typeof(Object));
439+
}
428440
il.Emit(OpCodes.Ldloc_0);
429-
il.Emit(OpCodes.Ldc_I4, i);
430-
il.Emit(OpCodes.Ldarg, i + 1);
431-
if (argTypes[i].IsValueType)
432-
il.Emit(OpCodes.Box, argTypes[i]);
433-
il.Emit(OpCodes.Stelem, typeof(Object));
434-
}
435-
il.Emit(OpCodes.Ldloc_0);
436-
if (returnType == typeof(void))
437-
{
438-
il.Emit(OpCodes.Call, typeof(PythonDerivedType).GetMethod("InvokeMethodVoid"));
439-
}
440-
else
441-
{
442-
il.Emit(OpCodes.Call, typeof(PythonDerivedType).GetMethod("InvokeMethod").MakeGenericMethod(returnType));
441+
if (returnType == typeof(void))
442+
{
443+
il.Emit(OpCodes.Call, typeof(PythonDerivedType).GetMethod("InvokeMethodVoid"));
444+
}
445+
else
446+
{
447+
il.Emit(OpCodes.Call, typeof(PythonDerivedType).GetMethod("InvokeMethod").MakeGenericMethod(returnType));
448+
}
449+
il.Emit(OpCodes.Ret);
443450
}
444-
il.Emit(OpCodes.Ret);
445451
}
446452

447453
/// <summary>
@@ -460,47 +466,49 @@ private static void AddPythonProperty(string propertyName, PyObject func, TypeBu
460466
MethodAttributes.HideBySig |
461467
MethodAttributes.SpecialName;
462468

463-
PyObject pyPropertyType = func.GetAttr("_clr_property_type_");
464-
Type propertyType = pyPropertyType.AsManagedObject(typeof(Type)) as Type;
465-
if (propertyType == null)
466-
throw new ArgumentException("_clr_property_type must be a CLR type");
467-
468-
PropertyBuilder propertyBuilder = typeBuilder.DefineProperty(propertyName,
469-
PropertyAttributes.None,
470-
propertyType,
471-
null);
472-
473-
if (func.HasAttr("fget") && func.GetAttr("fget").IsTrue())
469+
using (PyObject pyPropertyType = func.GetAttr("_clr_property_type_"))
474470
{
475-
MethodBuilder methodBuilder = typeBuilder.DefineMethod("get_" + propertyName,
476-
methodAttribs,
477-
propertyType,
478-
null);
479-
480-
ILGenerator il = methodBuilder.GetILGenerator();
481-
il.Emit(OpCodes.Ldarg_0);
482-
il.Emit(OpCodes.Ldstr, propertyName);
483-
il.Emit(OpCodes.Call, typeof(PythonDerivedType).GetMethod("InvokeGetProperty").MakeGenericMethod(propertyType));
484-
il.Emit(OpCodes.Ret);
471+
Type propertyType = pyPropertyType.AsManagedObject(typeof(Type)) as Type;
472+
if (propertyType == null)
473+
throw new ArgumentException("_clr_property_type must be a CLR type");
485474

486-
propertyBuilder.SetGetMethod(methodBuilder);
487-
}
488-
489-
if (func.HasAttr("fset") && func.GetAttr("fset").IsTrue())
490-
{
491-
MethodBuilder methodBuilder = typeBuilder.DefineMethod("set_" + propertyName,
492-
methodAttribs,
493-
null,
494-
new Type[]{propertyType});
475+
PropertyBuilder propertyBuilder = typeBuilder.DefineProperty(propertyName,
476+
PropertyAttributes.None,
477+
propertyType,
478+
null);
495479

496-
ILGenerator il = methodBuilder.GetILGenerator();
497-
il.Emit(OpCodes.Ldarg_0);
498-
il.Emit(OpCodes.Ldstr, propertyName);
499-
il.Emit(OpCodes.Ldarg_1);
500-
il.Emit(OpCodes.Call, typeof(PythonDerivedType).GetMethod("InvokeSetProperty").MakeGenericMethod(propertyType));
501-
il.Emit(OpCodes.Ret);
480+
if (func.HasAttr("fget") && func.GetAttr("fget").IsTrue())
481+
{
482+
MethodBuilder methodBuilder = typeBuilder.DefineMethod("get_" + propertyName,
483+
methodAttribs,
484+
propertyType,
485+
null);
486+
487+
ILGenerator il = methodBuilder.GetILGenerator();
488+
il.Emit(OpCodes.Ldarg_0);
489+
il.Emit(OpCodes.Ldstr, propertyName);
490+
il.Emit(OpCodes.Call, typeof(PythonDerivedType).GetMethod("InvokeGetProperty").MakeGenericMethod(propertyType));
491+
il.Emit(OpCodes.Ret);
492+
493+
propertyBuilder.SetGetMethod(methodBuilder);
494+
}
502495

503-
propertyBuilder.SetSetMethod(methodBuilder);
496+
if (func.HasAttr("fset") && func.GetAttr("fset").IsTrue())
497+
{
498+
MethodBuilder methodBuilder = typeBuilder.DefineMethod("set_" + propertyName,
499+
methodAttribs,
500+
null,
501+
new Type[]{propertyType});
502+
503+
ILGenerator il = methodBuilder.GetILGenerator();
504+
il.Emit(OpCodes.Ldarg_0);
505+
il.Emit(OpCodes.Ldstr, propertyName);
506+
il.Emit(OpCodes.Ldarg_1);
507+
il.Emit(OpCodes.Call, typeof(PythonDerivedType).GetMethod("InvokeSetProperty").MakeGenericMethod(propertyType));
508+
il.Emit(OpCodes.Ret);
509+
510+
propertyBuilder.SetSetMethod(methodBuilder);
511+
}
504512
}
505513
}
506514

@@ -681,25 +689,16 @@ public static T InvokeGetProperty<T>(IPythonDerivedType obj, string propertyName
681689
if (null == self)
682690
throw new NullReferenceException("Instance must be specified when getting a property");
683691

684-
List<PyObject> disposeList = new List<PyObject>();
685692
IntPtr gs = Runtime.PyGILState_Ensure();
686693
try
687694
{
688695
Runtime.Incref(self.pyHandle);
689-
PyObject pyself = new PyObject(self.pyHandle);
690-
disposeList.Add(pyself);
691-
692-
PyObject pyvalue = pyself.GetAttr(propertyName);
693-
disposeList.Add(pyvalue);
694-
return (T)pyvalue.AsManagedObject(typeof(T));
696+
using (PyObject pyself = new PyObject(self.pyHandle))
697+
using (PyObject pyvalue = pyself.GetAttr(propertyName))
698+
return (T)pyvalue.AsManagedObject(typeof(T));
695699
}
696700
finally
697701
{
698-
foreach (PyObject x in disposeList)
699-
{
700-
if (x != null)
701-
x.Dispose();
702-
}
703702
Runtime.PyGILState_Release(gs);
704703
}
705704
}
@@ -712,26 +711,16 @@ public static void InvokeSetProperty<T>(IPythonDerivedType obj, string propertyN
712711
if (null == self)
713712
throw new NullReferenceException("Instance must be specified when setting a property");
714713

715-
List<PyObject> disposeList = new List<PyObject>();
716714
IntPtr gs = Runtime.PyGILState_Ensure();
717715
try
718716
{
719717
Runtime.Incref(self.pyHandle);
720-
PyObject pyself = new PyObject(self.pyHandle);
721-
disposeList.Add(pyself);
722-
723-
PyObject pyvalue = new PyObject(Converter.ToPythonImplicit(value));
724-
disposeList.Add(pyvalue);
725-
726-
pyself.SetAttr(propertyName, pyvalue);
718+
using (PyObject pyself = new PyObject(self.pyHandle))
719+
using (PyObject pyvalue = new PyObject(Converter.ToPythonImplicit(value)))
720+
pyself.SetAttr(propertyName, pyvalue);
727721
}
728722
finally
729723
{
730-
foreach (PyObject x in disposeList)
731-
{
732-
if (x != null)
733-
x.Dispose();
734-
}
735724
Runtime.PyGILState_Release(gs);
736725
}
737726
}

‎src/runtime/converter.cs

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -175,17 +175,19 @@ internal static IntPtr ToPython(Object value, Type type) {
175175

176176
default:
177177
if (value is IEnumerable) {
178-
var resultlist = new PyList();
179-
foreach (object o in (IEnumerable)value) {
180-
resultlist.Append(new PyObject(ToPython(o, o.GetType())));
178+
using (var resultlist = new PyList()) {
179+
foreach (object o in (IEnumerable)value) {
180+
using (var p = new PyObject(ToPython(o, o.GetType())))
181+
resultlist.Append(p);
182+
}
183+
Runtime.Incref(resultlist.Handle);
184+
return resultlist.Handle;
181185
}
182-
Runtime.Incref(resultlist.Handle);
183-
return resultlist.Handle;
184186
}
185-
186187
result = CLRObject.GetInstHandle(value, type);
187188
return result;
188189
}
190+
189191
}
190192

191193

0 commit comments

Comments
 (0)