- Notifications
You must be signed in to change notification settings - Fork768
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 to927f980Comparelostmsu 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.
|
lostmsu commentedNov 11, 2020
@amos402 I could see it being freed by one of |
amos402 commentedNov 13, 2020
pythonnet/src/runtime/classderived.cs Lines 77 to 80 in927f980
It set by a weak GCHandle instead after it freed. |
tminka commentedDec 17, 2020
… 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
MoveClrInstancesOnwershipToPythonDoes this close any currently open issues?
#1256
Checklist
Check all those that are applicable and complete.
AUTHORSCHANGELOG