- Notifications
You must be signed in to change notification settings - Fork756
Grab the GIL on shutdown when checking for exceptions#1832
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
The GIL isn't taken when being called from the DomainUnload handler.
| var pyGILState = PyGILState.PyGILState_UNLOCKED; | ||
| if (!Runtime.HostedInPython) | ||
| { | ||
| pyGILState = Runtime.PyGILState_Ensure(); | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
SincePy.GIL() no longer tries to force runtime initialization, I think the code would be easier to read with a simpleusing (Py.GIL()) { ... } around theif.
I don't think it is necessary to check if we are hosted in Python.Runtime.Shutdown further down does not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Another issue is that the code below throws when Python error is set. And thatthrow prevents the GIL release code to be reached making the GIL unreleasable. Would not be a problem withusing Py.GIL
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I'll adjust it accordingly s.t. we can tag rc1.
Uh oh!
There was an error while loading.Please reload this page.
What does this implement/fix? Explain your changes.
The GIL isn't taken when being called from the DomainUnload handler. Take it to avoid a crash when calling
Exceptions.ErrorOccurred()Does this close any currently open issues?
#1831
Checklist
Check all those that are applicable and complete.
AUTHORS