Fix illegal delegate usage #1328
Conversation
Cache the delegates to native code we've created so that when we need to call that delegate, we don't ask for a delegate from a native pointer (that is a delegate to managed code.)
|
Test failures are the same as the ones in the current master: https://github.com/pythonnet/pythonnet/runs/1571830014 |
src/runtime/nativecall.cs
Outdated
| if (!Interop.allocatedThunks.TryGetValue(fp, out d)) | ||
| { | ||
| // Use Marshal.GetDelegateForFunctionPointer<> directly after upgrade the framework | ||
| d = Marshal.GetDelegateForFunctionPointer(fp, typeof(T)); |
There was a problem hiding this comment.
Can you also apply the comment here? The generic function was introduced in .NET 4.5.1.
There was a problem hiding this comment.
Actually, maybe just remove the comment. You are not writing this into allocatedThunks right now, btw.
There was a problem hiding this comment.
Yes, I don't cache these because they truly point to unmanaged code. We should only cache the function pointers to managed code.
| // We don't cache this delegate because this is a pure delegate ot unmanaged. | ||
| d = Marshal.GetDelegateForFunctionPointer<T>(fp); | ||
| } | ||
| return (T)d; |
There was a problem hiding this comment.
You'd save a couple instructions with:
Delegate d; // no init
if (trygetvalue(out d)) {
return (T)d;
} else {
return Marshal.GetDelegate...(); // no cast
}
| Delegate d = null; | ||
| if (!Interop.allocatedThunks.TryGetValue(fp, out d)) |
There was a problem hiding this comment.
NIT: you don't need to assign an initial value to d. You could also do TryGetValue(fp, out var d).
What does this implement/fix? Explain your changes.
Cache the delegates to native code we've created so that when we need to
call that delegate, we don't ask for a delegate from a native pointer
(that is a delegate to managed code.)
Does this close any currently open issues?
This fixes issue #1323 .
Any other comments?
This is needed in order for #1287 to pass the domain reload tests on Mono.
Checklist
Check all those that are applicable and complete.
AUTHORSCHANGELOG