- Notifications
You must be signed in to change notification settings - Fork748
Fixed crash in finalizer of CLR types defined in Python, that survive engine shutdown#1260
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
9498c07
to927f980
Comparelostmsu commentedOct 13, 2020 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Interesting. It appears, that the fix is legitimate, but the test for it causes segfault in soft shutdown mode with xplat. Which probably means it is caused by a separate issue :/ |
codecov-io commentedOct 13, 2020 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Codecov Report
@@ Coverage Diff @@## master #1260 +/- ##======================================= Coverage 86.25% 86.25% ======================================= Files 1 1 Lines 291 291 ======================================= Hits 251 251 Misses 40 40
Flags with carried forward coverage won't be shown.Click here to find out more. Continue to review full report at Codecov.
|
@amos402 I could see it being freed by one of |
pythonnet/src/runtime/classderived.cs Lines 77 to 80 in927f980
It set by a weak GCHandle instead after it freed. |
… engine shutdownpythonnet#1256pythonnet#1256During engine shutdown all links from Python to .NET instances are severed. If an instance of CLR class defined in Python survives the shutdown (for example, a reference is stored in static field) and later gets finalized, it will attempt to severe link again, which is an invalid operation.The fix is to check if the link has already been severed and skip that step during finalization.
… engine shutdown (pythonnet#1260)pythonnet#1256pythonnet#1256During engine shutdown all links from Python to .NET instances are severed. If an instance of CLR class defined in Python survives the shutdown (for example, a reference is stored in static field) and later gets finalized, it will attempt to severe link again, which is an invalid operation.The fix is to check if the link has already been severed and skip that step during finalization.
… engine shutdown (pythonnet#1260)pythonnet#1256pythonnet#1256During engine shutdown all links from Python to .NET instances are severed. If an instance of CLR class defined in Python survives the shutdown (for example, a reference is stored in static field) and later gets finalized, it will attempt to severe link again, which is an invalid operation.The fix is to check if the link has already been severed and skip that step during finalization.
What does this implement/fix? Explain your changes.
During engine shutdown all links from Python to .NET instances are severed. If an instance of CLR class defined in Python survives the shutdown (for example, a reference is stored in static field) and later gets finalized, it will attempt to severe link again, which is an invalid operation.
The fix is to check if the link has already been severed and skip that step during finalization.
Additionally in this change: refactored
MoveClrInstancesOnwershipToPython
Does this close any currently open issues?
#1256
Checklist
Check all those that are applicable and complete.
AUTHORS
CHANGELOG