Skip to content

Commit f93e9a2

Browse files
lostmsulostmsu
committed
remove risky finalization code from Dispatcher, and use reference types
1 parent 12defa7 commit f93e9a2

File tree

4 files changed

+33
-64
lines changed

4 files changed

+33
-64
lines changed

‎src/runtime/converter.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,8 @@ internal static IntPtr ToPython<T>(T value)
112112

113113
internal static NewReference ToPythonReference<T>(T value)
114114
=> NewReference.DangerousFromPointer(ToPython(value, typeof(T)));
115+
internal static NewReference ToPythonReference(object value, Type type)
116+
=> NewReference.DangerousFromPointer(ToPython(value, type));
115117

116118
private static readonly Func<object, bool> IsTransparentProxy = GetIsTransparentProxy();
117119

‎src/runtime/delegatemanager.cs

Lines changed: 26 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -29,21 +29,6 @@ public DelegateManager()
2929
dispatch = basetype.GetMethod("Dispatch");
3030
}
3131

32-
/// <summary>
33-
/// Given a true delegate instance, return the PyObject handle of the
34-
/// Python object implementing the delegate (or IntPtr.Zero if the
35-
/// delegate is not implemented in Python code.
36-
/// </summary>
37-
public IntPtr GetPythonHandle(Delegate d)
38-
{
39-
if (d?.Target is Dispatcher)
40-
{
41-
var disp = (Dispatcher)d.Target;
42-
return disp.target;
43-
}
44-
return IntPtr.Zero;
45-
}
46-
4732
/// <summary>
4833
/// GetDispatcher is responsible for creating a class that provides
4934
/// an appropriate managed callback method for a given delegate type.
@@ -224,41 +209,15 @@ A possible alternate strategy would be to create custom subclasses
224209

225210
public class Dispatcher
226211
{
227-
public IntPtr target;
228-
public Type dtype;
229-
private bool _disposed = false;
230-
private bool _finalized = false;
212+
readonly PyObject target;
213+
readonly Type dtype;
231214

232215
public Dispatcher(IntPtr target, Type dtype)
233216
{
234-
Runtime.XIncref(target);
235-
this.target = target;
217+
this.target = new PyObject(new BorrowedReference(target));
236218
this.dtype = dtype;
237219
}
238220

239-
~Dispatcher()
240-
{
241-
if (_finalized || _disposed)
242-
{
243-
return;
244-
}
245-
_finalized = true;
246-
Finalizer.Instance.AddFinalizedObject(ref target);
247-
}
248-
249-
public void Dispose()
250-
{
251-
if (_disposed)
252-
{
253-
return;
254-
}
255-
_disposed = true;
256-
Runtime.XDecref(target);
257-
target = IntPtr.Zero;
258-
dtype = null;
259-
GC.SuppressFinalize(this);
260-
}
261-
262221
public object Dispatch(object[] args)
263222
{
264223
IntPtr gs = PythonEngine.AcquireLock();
@@ -280,26 +239,36 @@ private object TrueDispatch(object[] args)
280239
{
281240
MethodInfo method = dtype.GetMethod("Invoke");
282241
ParameterInfo[] pi = method.GetParameters();
283-
IntPtr pyargs = Runtime.PyTuple_New(pi.Length);
284242
Type rtype = method.ReturnType;
285243

286-
for (var i = 0; i < pi.Length; i++)
244+
NewReference op;
245+
using (var pyargs = NewReference.DangerousFromPointer(Runtime.PyTuple_New(pi.Length)))
287246
{
288-
// Here we own the reference to the Python value, and we
289-
// give the ownership to the arg tuple.
290-
IntPtr arg = Converter.ToPython(args[i], pi[i].ParameterType);
291-
Runtime.PyTuple_SetItem(pyargs, i, arg);
292-
}
247+
for (var i = 0; i < pi.Length; i++)
248+
{
249+
// Here we own the reference to the Python value, and we
250+
// give the ownership to the arg tuple.
251+
var arg = Converter.ToPythonReference(args[i], pi[i].ParameterType);
252+
if (arg.IsNull())
253+
{
254+
throw PythonException.ThrowLastAsClrException();
255+
}
256+
int res = Runtime.PyTuple_SetItem(pyargs, i, arg.Steal());
257+
if (res != 0)
258+
{
259+
throw PythonException.ThrowLastAsClrException();
260+
}
261+
}
293262

294-
IntPtr op = Runtime.PyObject_Call(target, pyargs, IntPtr.Zero);
295-
Runtime.XDecref(pyargs);
263+
op = Runtime.PyObject_Call(target.Reference, pyargs, BorrowedReference.Null);
264+
}
296265

297-
if (op == IntPtr.Zero)
266+
if (op.IsNull())
298267
{
299268
throw PythonException.ThrowLastAsClrException();
300269
}
301270

302-
try
271+
using (op)
303272
{
304273
int byRefCount = pi.Count(parameterInfo => parameterInfo.ParameterType.IsByRef);
305274
if (byRefCount > 0)
@@ -339,7 +308,7 @@ private object TrueDispatch(object[] args)
339308
Type t = pi[i].ParameterType;
340309
if (t.IsByRef)
341310
{
342-
IntPtr item = Runtime.PyTuple_GetItem(op, index++);
311+
BorrowedReference item = Runtime.PyTuple_GetItem(op, index++);
343312
if (!Converter.ToManaged(item, t, out object newArg, true))
344313
{
345314
Exceptions.RaiseTypeError($"The Python function returned a tuple where element {i} was not {t.GetElementType()} (the out parameter type)");
@@ -352,7 +321,7 @@ private object TrueDispatch(object[] args)
352321
{
353322
return null;
354323
}
355-
IntPtr item0 = Runtime.PyTuple_GetItem(op, 0);
324+
BorrowedReference item0 = Runtime.PyTuple_GetItem(op, 0);
356325
if (!Converter.ToManaged(item0, rtype, out object result0, true))
357326
{
358327
Exceptions.RaiseTypeError($"The Python function returned a tuple where element 0 was not {rtype} (the return type)");
@@ -397,10 +366,6 @@ private object TrueDispatch(object[] args)
397366

398367
return result;
399368
}
400-
finally
401-
{
402-
Runtime.XDecref(op);
403-
}
404369
}
405370
}
406371
}

‎src/runtime/importhook.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ static void SetupImportHook()
120120
var mod_dict = Runtime.PyModule_GetDict(import_hook_module);
121121
// reference not stolen due to overload incref'ing for us.
122122
Runtime.PyTuple_SetItem(args, 1, mod_dict);
123-
Runtime.PyObject_Call(exec, args, default);
123+
Runtime.PyObject_Call(exec, args, default).Dispose();
124124
// Set as a sub-module of clr.
125125
if(Runtime.PyModule_AddObject(ClrModuleReference, "loader", import_hook_module.DangerousGetAddress()) != 0)
126126
{

‎src/runtime/runtime.cs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1013,6 +1013,8 @@ internal static IntPtr PyObject_Type(IntPtr op)
10131013
internal static NewReference PyObject_Type(BorrowedReference o)
10141014
=> Delegates.PyObject_Type(o);
10151015

1016+
internal static string PyObject_GetTypeName(BorrowedReference op)
1017+
=> PyObject_GetTypeName(op.DangerousGetAddress());
10161018
internal static string PyObject_GetTypeName(IntPtr op)
10171019
{
10181020
IntPtr pyType = PyObject_TYPE(op);
@@ -1097,8 +1099,8 @@ internal static IntPtr PyObject_GetAttr(IntPtr pointer, IntPtr name)
10971099

10981100

10991101
internal static IntPtr PyObject_Call(IntPtr pointer, IntPtr args, IntPtr kw) => Delegates.PyObject_Call(pointer, args, kw);
1100-
internal static IntPtr PyObject_Call(BorrowedReference pointer, BorrowedReference args, BorrowedReference kw)
1101-
=> Delegates.PyObject_Call(pointer.DangerousGetAddress(), args.DangerousGetAddress(), kw.DangerousGetAddressOrNull());
1102+
internal static NewReference PyObject_Call(BorrowedReference pointer, BorrowedReference args, BorrowedReference kw)
1103+
=> NewReference.DangerousFromPointer(Delegates.PyObject_Call(pointer.DangerousGetAddress(), args.DangerousGetAddress(), kw.DangerousGetAddressOrNull()));
11021104

11031105

11041106
internal static NewReference PyObject_CallObject(BorrowedReference callable, BorrowedReference args) => Delegates.PyObject_CallObject(callable, args);

0 commit comments

Comments
 (0)