I am using a custom encoder defined in Python to work around the conversion of IList to Python lists (see #963 (comment)):
# Prevent Python.NET from converting IList to Python listsfromPython.RuntimeimportPyObjectConversionsfromPython.Runtime.CodecsimportRawProxyEncoderclassListAsRawEncoder(RawProxyEncoder):
__namespace__='Python.Runtime.Codecs'defCanEncode(self, clr_type):
returnclr_type.Name=='IList`1'andclr_type.Namespace=='System.Collections.Generic'list_encoder=ListAsRawEncoder()
PyObjectConversions.RegisterEncoder(list_encoder)
This works fine until I have called Python.Runtime.PythonEngine.Shutdown, and GC kicks in. Then there is this exception:
Unhandled exception
Exception thrown:
System.InvalidOperationException: Handle is not initialized.
at System.Runtime.InteropServices.GCHandle.Free()
at Python.Runtime.PythonDerivedType.Finalize(IPythonDerivedType obj)
at Python.Runtime.Codecs.ListAsRawEncoder.Finalize()
The crash started happening after #958 was merged.
@amos402 this appears to be because MoveClrInstancesOnwershipToPython calls Free on GCHandle, and does not prevent the object from being subsequently finalized.
MoveClrInstancesOnwershipToPython didn't deal with the IPythonDerivedType, the gcHandle shouldn't be free twice, it only is called in Finalize.
I encountered some kinds of errors by subtypes too when I writing #1267, but I can't remember is this or not, if you can reproduce it reliably, can you test on it?
But this reminds me, currently, all the shutdown modes won't able to handle the IPythonDerivedType, because it would create dynamic CLR types. This is a defect.
Actually, the feature of creating a sub type inherit from c# type is implementable without creating dynamic types, the current implementation is not as good well.
@danabr this PR has a monkey-patch for the issue that you are experiencing: #1260
Unfortunately, I don't have time to produce proper patch. One needs to spend some to understand what are the expected scenarios specifying the lifetime of PythonDerivedType class. Ideally, a IDisposable-like semantics should be implemented, with a guarantee, that multiple calls to Dispose are allowed. Currently there is no single Dispose. Instead, there's a weirdly attached finalizer in Finalize, and one of tp_clear static methods, that try to clean that handle up.
The issue reproduces pretty reliably in https://github.com/losttech/pythonnet/tree/bugs/PythonDerivedCrashFixes . In fact I think to reproduce it one just needs to create a type in Python derived from a .NET type, then shut runtime down without explicitly disposing it, and start it back up so that finalizer could run.
Checking if GCHandle has already been deallocated like in losttech@8045b5f does not appear to be sufficient.
I can not wrap my head around the mechanics of ClassDerivedObject and PythonDerivedType-powered instances. If nobody can fix the issue as it is, I will attempt to rewrite the ownership model of everything derived from ManagedType (which applies to ClassDerivedObject).
UPD. To reproduce this issue, you can now re-enable this test:
Activity
lostmsu commented on Oct 12, 2020
@danabr if you can reproduce this reliably, can you try to figure out the stack trace for the preceding GCHandle.Free call? Or maybe make a test case?
lostmsu commented on Oct 12, 2020
@danabr nvm, I am able to reproduce the issue.
lostmsu commented on Oct 12, 2020
@amos402 this appears to be because MoveClrInstancesOnwershipToPython calls Free on GCHandle, and does not prevent the object from being subsequently finalized.
fixed crash in finalizer of CLR types defined in Python, that survive…
fixed crash in finalizer of CLR types defined in Python, that survive…
amos402 commented on Oct 22, 2020
MoveClrInstancesOnwershipToPython didn't deal with the IPythonDerivedType, the gcHandle shouldn't be free twice, it only is called in Finalize.
I encountered some kinds of errors by subtypes too when I writing #1267, but I can't remember is this or not, if you can reproduce it reliably, can you test on it?
But this reminds me, currently, all the shutdown modes won't able to handle the IPythonDerivedType, because it would create dynamic CLR types. This is a defect.
Actually, the feature of creating a sub type inherit from c# type is implementable without creating dynamic types, the current implementation is not as good well.
danabr commented on Nov 17, 2020
@amos402 : I tested the test case provided in this issue on #1267, and it still fails the same way.
What can I do to help debugging this? This issue currently prevents me from using pythonnet/master, so I would be happy to find a way to solve it.
lostmsu commented on Nov 20, 2020
@danabr this PR has a monkey-patch for the issue that you are experiencing: #1260
Unfortunately, I don't have time to produce proper patch. One needs to spend some to understand what are the expected scenarios specifying the lifetime of PythonDerivedType class. Ideally, a IDisposable-like semantics should be implemented, with a guarantee, that multiple calls to Dispose are allowed. Currently there is no single Dispose. Instead, there's a weirdly attached finalizer in Finalize, and one of tp_clear static methods, that try to clean that handle up.
fixed crash in finalizer of CLR types defined in Python, that survive…
fixed crash in finalizer of CLR types defined in Python, that survive…
fixed crash in finalizer of CLR types defined in Python, that survive…
fixed crash in finalizer of CLR types defined in Python, that survive…
fixed crash in finalizer of CLR types defined in Python, that survive…
disabled PythonRegisteredDecoder_NoStackOverflowOnSystemType test due…
lostmsu commented on Mar 31, 2021
@amos402 @BadSingleton
The issue reproduces pretty reliably in https://github.com/losttech/pythonnet/tree/bugs/PythonDerivedCrashFixes . In fact I think to reproduce it one just needs to create a type in Python derived from a .NET type, then shut runtime down without explicitly disposing it, and start it back up so that finalizer could run.
Checking if GCHandle has already been deallocated like in losttech@8045b5f does not appear to be sufficient.
I can not wrap my head around the mechanics of ClassDerivedObject and PythonDerivedType-powered instances. If nobody can fix the issue as it is, I will attempt to rewrite the ownership model of everything derived from ManagedType (which applies to ClassDerivedObject).
UPD. To reproduce this issue, you can now re-enable this test:
pythonnet/src/embed_tests/Codecs.cs
Line 301 in 23527d1
disabled PythonRegisteredDecoder_NoStackOverflowOnSystemType test due…
reenabled test for pythonnet#1256 : NoStackOverflowOnSystemType
reenabled test for #1256 : NoStackOverflowOnSystemType (#1943)